From e84297b87a6875213969ca04280747c4683c0047 Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Thu, 30 Jul 2026 12:28:29 +0200 Subject: [PATCH 01/28] feat(config): add config-file resolver for root and legacy locations Probe streamctl.config. at the invocation directory, then .streamctl/config., over c12's own SUPPORTED_EXTENSIONS. Root wins when both exist and one warning names both paths. Probing before any load is what keeps c12's .config/ fallbacks unreachable. --- src/config/resolve.ts | 99 +++++++++++++++++ test/resolve.test.ts | 249 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 348 insertions(+) create mode 100644 src/config/resolve.ts create mode 100644 test/resolve.test.ts diff --git a/src/config/resolve.ts b/src/config/resolve.ts new file mode 100644 index 0000000..50564e7 --- /dev/null +++ b/src/config/resolve.ts @@ -0,0 +1,99 @@ +import type { Logger } from "../logger"; +import { statSync } from "node:fs"; +import { relative, resolve, sep } from "node:path"; +import { SUPPORTED_EXTENSIONS } from "c12"; + +/** The resolved answer to "where does this repo's config live". */ +export interface ConfigFileLocation { + abs: string; + /** `cwd`-relative, POSIX-separated on every platform. Doubles as a git pathspec. */ + rel: string; + /** Which of the two supported locations matched. */ + source: "root" | "legacy"; +} + +/** Default location: `streamctl.config.` at the invocation directory. */ +export const CONFIG_FILE = "streamctl.config"; + +/** Legacy location, read indefinitely: `.streamctl/config.`. */ +export const LEGACY_CONFIG_FILE = ".streamctl/config"; + +/** + * c12's own extension list, in its own precedence order (note `.js` precedes `.ts`). + * Imported rather than restated so the two can never drift apart. + */ +const EXTENSIONS: readonly string[] = SUPPORTED_EXTENSIONS; + +/** + * A spelling crossed with every supported extension, in c12's order. Exported only so + * a test can pin the candidate list to c12's export. + * + * @internal + */ +export function configCandidates(spelling: string): string[] { + return EXTENSIONS.map(ext => `${spelling}${ext}`); +} + +/** + * `isFile`, not `existsSync`: a *directory* named `streamctl.config.ts` would pass an + * existence check and be returned as a root hit, shadowing a real legacy config and + * handing `load.ts` an `abs` that c12 will not corroborate. + * + * `throwIfNoEntry` suppresses ENOENT only, so EACCES on an unreadable parent still + * throws; the catch is what keeps `resolveConfigFile` total. + */ +function isFile(abs: string): boolean { + try { + return statSync(abs, { throwIfNoEntry: false })?.isFile() ?? false; + } catch { + return false; + } +} + +function probe(cwd: string, spelling: string): string | null { + for (const candidate of configCandidates(spelling)) { + const abs = resolve(cwd, candidate); + if (isFile(abs)) { + return abs; + } + } + return null; +} + +/** + * Locate the repo's config, root location first. Returns `null` when neither location + * holds one; never throws. + * + * The probe runs *before* anything is loaded, and that ordering is what keeps c12's + * `.config/` fallbacks out: `tryResolve` exhausts every extension on the primary path + * before trying `.config/` (`c12/dist/index.mjs:334-343`), so handing the loader a + * spelling whose file is known to exist makes those branches unreachable. Probing + * after a load, or loading speculatively, silently re-admits `.config/` paths. + * + * `logger` is optional with no `stderrLogger` fallback — a deliberate deviation from + * the house `opts.logger ?? stderrLogger` default, because `init` calls this without a + * logger precisely to stay quiet ahead of its `ALREADY_INITIALIZED` failure. + * + * `async` with nothing awaited is deliberate — the signature stays `Promise`-returning + * so `init`'s guard and `load.ts` keep `await`ing it, and a move to `fs.promises` is + * not a breaking change (`50_api.md`). + */ +export async function resolveConfigFile(cwd: string, logger?: Logger): Promise { + const rootAbs = probe(cwd, CONFIG_FILE); + // Probed even on a root hit: it is the only signal for the both-present warning. + const legacyAbs = probe(cwd, LEGACY_CONFIG_FILE); + const abs = rootAbs ?? legacyAbs; + if (abs === null) { + return null; + } + + const toRel = (path: string): string => relative(cwd, path).split(sep).join("/"); + + if (rootAbs !== null && legacyAbs !== null) { + logger?.warn( + `streamctl: both ${toRel(rootAbs)} and ${toRel(legacyAbs)} exist; using ${toRel(rootAbs)} and ignoring ${toRel(legacyAbs)}.`, + ); + } + + return { abs, rel: toRel(abs), source: rootAbs !== null ? "root" : "legacy" }; +} diff --git a/test/resolve.test.ts b/test/resolve.test.ts new file mode 100644 index 0000000..459ff32 --- /dev/null +++ b/test/resolve.test.ts @@ -0,0 +1,249 @@ +import { chmodSync, existsSync, mkdirSync, mkdtempSync, realpathSync, rmSync, statSync } from "node:fs"; +import { chmod, mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { isAbsolute, join, resolve } from "node:path"; +import { pathToFileURL } from "node:url"; +import { SUPPORTED_EXTENSIONS } from "c12"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { CONFIG_FILE, configCandidates, LEGACY_CONFIG_FILE, resolveConfigFile } from "../src/config/resolve"; + +/** + * Does `chmod 0o000` on a directory actually revoke traversal here? Windows can't + * revoke it that way and root ignores the bit outright — in both cases the stat + * succeeds and the EACCES expectation would fail. + */ +function chmodCanRevokeTraversal(): boolean { + const dir = mkdtempSync(join(tmpdir(), "streamctl-chmodprobe-")); + const blocked = join(dir, "blocked"); + try { + mkdirSync(blocked); + chmodSync(blocked, 0o000); + statSync(join(blocked, "child"), { throwIfNoEntry: false }); + return false; // still traversable at 0o000, so: Windows ACLs, or running as root + } catch { + return true; + } finally { + chmodSync(blocked, 0o755); + rmSync(dir, { recursive: true, force: true }); + } +} + +const canRevokeTraversal = chmodCanRevokeTraversal(); + +let root: string; +let warnings: string[]; +const logger = { warn: (message: string) => warnings.push(message) }; + +beforeEach(async () => { + // realpath'd at creation: Windows runners hand back an 8.3 short-name temp path and + // exsolve may realpath, so an unresolved `root` would make `abs` disagree with what + // c12 later reports for the same file. + root = realpathSync(await mkdtemp(join(tmpdir(), "streamctl-resolve-"))); + warnings = []; +}); + +afterEach(async () => { + await rm(root, { recursive: true, force: true }); +}); + +async function writeRootConfig(ext = ".ts", body = "export default {}\n"): Promise { + await writeFile(join(root, `streamctl.config${ext}`), body); +} + +async function writeLegacyConfig(ext = ".ts", body = "export default {}\n"): Promise { + await mkdir(join(root, ".streamctl"), { recursive: true }); + await writeFile(join(root, ".streamctl", `config${ext}`), body); +} + +/** A config whose module body touches the filesystem when evaluated. */ +function sideEffectConfig(sentinel: string): string { + return `import { writeFileSync } from "node:fs";\nwriteFileSync(${JSON.stringify(sentinel)}, "");\nexport default {}\n`; +} + +describe("resolveConfigFile", () => { + it("resolves a root config", async () => { + await writeRootConfig(); + + const location = await resolveConfigFile(root, logger); + + expect(location).toEqual({ + abs: join(root, "streamctl.config.ts"), + rel: "streamctl.config.ts", + source: "root", + }); + expect(warnings).toEqual([]); + }); + + it("resolves a legacy config with a POSIX-separated `rel`", async () => { + await writeLegacyConfig(); + + const location = await resolveConfigFile(root, logger); + + expect(location?.source).toBe("legacy"); + expect(location?.rel).toBe(".streamctl/config.ts"); + expect(location?.rel).not.toContain("\\"); + expect(location?.abs).toBe(join(root, ".streamctl", "config.ts")); + expect(warnings).toEqual([]); + }); + + it("returns null when neither location holds a config", async () => { + expect(await resolveConfigFile(root, logger)).toBeNull(); + expect(warnings).toEqual([]); + }); + + it("`abs` is absolute and agrees with `rel`", async () => { + await writeLegacyConfig(); + + const location = await resolveConfigFile(root, logger); + + expect(isAbsolute(location?.abs ?? "")).toBe(true); + expect(location?.abs).toBe(resolve(root, location?.rel ?? "")); + }); + + describe("both locations present", () => { + beforeEach(async () => { + await writeRootConfig(); + await writeLegacyConfig(); + }); + + it("resolves the root config and warns exactly once", async () => { + const location = await resolveConfigFile(root, logger); + + expect(location?.source).toBe("root"); + expect(location?.rel).toBe("streamctl.config.ts"); + expect(warnings).toHaveLength(1); + }); + + it("names both paths and which one is in use", async () => { + await resolveConfigFile(root, logger); + + // Substrings, never the whole sentence: the exact wording is still open, and a + // verbatim assertion would make changing it expensive. + const [warning] = warnings; + expect(warning).toContain("streamctl.config.ts"); + expect(warning).toContain(".streamctl/config.ts"); + expect(warning).toContain("using streamctl.config.ts"); + expect(warning).toContain("ignoring .streamctl/config.ts"); + }); + + it("emits nothing when no logger is passed", async () => { + const location = await resolveConfigFile(root); + + expect(location?.source).toBe("root"); + expect(warnings).toEqual([]); + }); + + it("evaluates neither config module", async () => { + // Structural: the resolver only stats. Pinned anyway, because a return to a + // speculative-`loadConfig` design would evaluate the ignored config silently, + // and this is the assertion that would catch it. + const rootSentinel = join(root, "root-evaluated"); + const legacySentinel = join(root, "legacy-evaluated"); + await writeRootConfig(".ts", sideEffectConfig(rootSentinel)); + await writeLegacyConfig(".ts", sideEffectConfig(legacySentinel)); + + await resolveConfigFile(root, logger); + + expect(existsSync(rootSentinel)).toBe(false); + expect(existsSync(legacySentinel)).toBe(false); + + // Proof the fixture is not inert: evaluated directly, the same body does write. + // Without this the two assertions above would also pass on a broken fixture. + // Native ESM import of an out-of-root file — the only one in the suite; if this + // line ever fails on CI it is a harness issue, not a resolver bug. + const proofSentinel = join(root, "proof-evaluated"); + const proofModule = join(root, "proof.config.mjs"); + await writeFile(proofModule, sideEffectConfig(proofSentinel)); + await import(pathToFileURL(proofModule).href); + expect(existsSync(proofSentinel)).toBe(true); + }); + }); + + describe("extensions", () => { + it.each([".ts", ".mjs", ".mts", ".js", ".json", ".yaml"])("resolves a root config with %s", async (ext) => { + await writeRootConfig(ext); + + const location = await resolveConfigFile(root, logger); + + expect(location?.rel).toBe(`streamctl.config${ext}`); + expect(location?.source).toBe("root"); + }); + + it("prefers .js over .ts at the same location, per c12's order", async () => { + await writeRootConfig(".ts"); + await writeRootConfig(".js"); + + const location = await resolveConfigFile(root, logger); + + expect(location?.rel).toBe("streamctl.config.js"); + // Same-location shadowing is not the both-present case. + expect(warnings).toEqual([]); + }); + + it("builds one candidate per c12 extension, in c12's order", async () => { + expect(SUPPORTED_EXTENSIONS.length).toBeGreaterThan(0); + // Length and order, not just non-empty: a builder that ignored the import would + // pass a bare non-empty check while silently disabling the whole probe. + expect(configCandidates(CONFIG_FILE)).toEqual(SUPPORTED_EXTENSIONS.map(ext => `streamctl.config${ext}`)); + expect(configCandidates(LEGACY_CONFIG_FILE)).toHaveLength(SUPPORTED_EXTENSIONS.length); + }); + }); + + describe("directory shaped like a config", () => { + // Distinct from `streamctl.config/index.ts`, which c12 would accept via + // `suffixes: ["", "/index"]` and the probe deliberately does not: here the + // *candidate itself* is a directory, so an `existsSync` probe would call it a hit. + it("is not a hit", async () => { + await mkdir(join(root, "streamctl.config.ts")); + + expect(await resolveConfigFile(root, logger)).toBeNull(); + expect(warnings).toEqual([]); + }); + + it("does not shadow a legacy config or trigger the warning", async () => { + await mkdir(join(root, "streamctl.config.ts")); + await writeLegacyConfig(); + + const location = await resolveConfigFile(root, logger); + + expect(location?.source).toBe("legacy"); + expect(warnings).toEqual([]); + }); + }); + + describe("never throws", () => { + it("does not throw for an empty cwd", async () => { + // `resolve("", …)` falls back to `process.cwd()`, so what this reads depends on + // the suite's working directory. Assert the contract, not this repo's contents. + const location = await resolveConfigFile("", logger); + + if (location !== null) { + expect(isAbsolute(location.abs)).toBe(true); + expect(location.rel).not.toContain("\\"); + } + }); + + it("returns null for a non-existent cwd", async () => { + expect(await resolveConfigFile(join(root, "nope"), logger)).toBeNull(); + }); + + it("returns null when cwd is a file", async () => { + const file = join(root, "package.json"); + await writeFile(file, "{}\n"); + + expect(await resolveConfigFile(file, logger)).toBeNull(); + }); + + it.skipIf(!canRevokeTraversal)("returns null when cwd is unreadable", async () => { + // `throwIfNoEntry: false` suppresses ENOENT only, so this path really does raise + // EACCES inside the probe. + const blocked = join(root, "blocked"); + await mkdir(blocked); + await chmod(blocked, 0o000); + + expect(await resolveConfigFile(blocked, logger)).toBeNull(); + + await chmod(blocked, 0o755); // restore so cleanup can recurse + }); + }); +}); From 82d922be3c6de9caeb35e92db1a2c1e57a6707f0 Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Thu, 30 Jul 2026 12:59:27 +0200 Subject: [PATCH 02/28] test: pin the .config/ exclusion and the directory-config divergence Nothing under .config/ resolves, and a repo holding both a .config/ file and a legacy config still reads the legacy one. The exclusion is structural, so these characterization tests are its only guard. --- test/resolve.test.ts | 88 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 86 insertions(+), 2 deletions(-) diff --git a/test/resolve.test.ts b/test/resolve.test.ts index 459ff32..d229de1 100644 --- a/test/resolve.test.ts +++ b/test/resolve.test.ts @@ -1,7 +1,7 @@ import { chmodSync, existsSync, mkdirSync, mkdtempSync, realpathSync, rmSync, statSync } from "node:fs"; -import { chmod, mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { chmod, mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; -import { isAbsolute, join, resolve } from "node:path"; +import { dirname, isAbsolute, join, resolve } from "node:path"; import { pathToFileURL } from "node:url"; import { SUPPORTED_EXTENSIONS } from "c12"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; @@ -55,6 +55,13 @@ async function writeLegacyConfig(ext = ".ts", body = "export default {}\n"): Pro await writeFile(join(root, ".streamctl", `config${ext}`), body); } +/** Write a file under `.config/`, creating its parents. */ +async function writeConfigDirFile(rel: string, body = "export default {}\n"): Promise { + const abs = join(root, ".config", rel); + await mkdir(dirname(abs), { recursive: true }); + await writeFile(abs, body); +} + /** A config whose module body touches the filesystem when evaluated. */ function sideEffectConfig(sentinel: string): string { return `import { writeFileSync } from "node:fs";\nwriteFileSync(${JSON.stringify(sentinel)}, "");\nexport default {}\n`; @@ -211,6 +218,83 @@ describe("resolveConfigFile", () => { }); }); + /** + * Characterization tests for a deliberate decision: `90_questions.md`, "What should + * happen to c12's `.config/` directory probing? — **Suppress it**" (2026-07-30). + * + * The exclusion is structural, not checked — confirming an intended candidate exists + * before anything loads makes c12's `.config/` branches (`dist/index.mjs:313`) + * unreachable. So there is no rejection code to review, and nothing visibly breaks if + * a refactor undoes it. These tests are the only guard. A failure here means the + * probe-before-load ordering was lost; do not "fix" it by re-admitting `.config/`. + */ + describe(".config/ is deliberately not supported", () => { + it("does not resolve .config/streamctl.ts", async () => { + // Not a regression: under the pre-change `configFile: ".streamctl/config"` this + // never resolved either, because c12 probes `.config/.streamctl/config` and never + // `.config/streamctl`. Switching to the root spelling is what *would* have + // started resolving it — verified against c12 3.3.4. This prevents that. + await writeConfigDirFile("streamctl.ts"); + + expect(await resolveConfigFile(root, logger)).toBeNull(); + expect(warnings).toEqual([]); + }); + + it("does not resolve .config/streamctl.config.ts", async () => { + // Same as above: never resolved before, and would have started under the root + // spelling (`.replace(/\.config$/, "")` strips the suffix, so c12 probes + // `.config/streamctl`, and its third branch probes `.config/streamctl.config`). + await writeConfigDirFile("streamctl.config.ts"); + + expect(await resolveConfigFile(root, logger)).toBeNull(); + expect(warnings).toEqual([]); + }); + + it("does not resolve .config/.streamctl/config.ts", async () => { + // The one real break in this block: this path DOES resolve today, because + // `.streamctl/config` ends in `/config` (no dot), survives the `.replace`, and + // c12 probes `.config/.streamctl/config`. Removing it is intentional and is + // called out in the release notes (P03-T03). + await writeConfigDirFile(join(".streamctl", "config.ts")); + + expect(await resolveConfigFile(root, logger)).toBeNull(); + expect(warnings).toEqual([]); + }); + + it("resolves the legacy config when .config/streamctl.ts also exists", async () => { + // Compatibility, not exclusion, and the most important case here: this repo shape + // reads the legacy file today. A design delegating resolution to c12 would have + // flipped it to the `.config/` file — a silent behavior change in exactly the + // population promised byte-identical behavior. + await writeConfigDirFile("streamctl.ts", "export default { base: \"from-config-dir\" }\n"); + await writeLegacyConfig(".ts", "export default { base: \"from-legacy\" }\n"); + + const location = await resolveConfigFile(root, logger); + + expect(location?.source).toBe("legacy"); + expect(location?.rel).toBe(".streamctl/config.ts"); + // Read back through the resolved path, so this proves *which* file was selected + // rather than merely that something was. + expect(await readFile(location?.abs ?? "", "utf8")).toContain("from-legacy"); + expect(warnings).toEqual([]); + }); + }); + + describe("directory-shaped config via c12's /index suffix", () => { + it("does not resolve streamctl.config/index.ts", async () => { + // A real, intentional divergence: c12 accepts this form via + // `suffixes: ["", "/index"]` (`dist/index.mjs:338`) — verified, it loads — and the + // probe deliberately does not mirror it, because a directory-shaped config is + // outside the two-locations promise. Distinct from the `streamctl.config.ts`-as-a + // -directory case above, where the candidate itself is the directory. + await mkdir(join(root, "streamctl.config")); + await writeFile(join(root, "streamctl.config", "index.ts"), "export default {}\n"); + + expect(await resolveConfigFile(root, logger)).toBeNull(); + expect(warnings).toEqual([]); + }); + }); + describe("never throws", () => { it("does not throw for an empty cwd", async () => { // `resolve("", …)` falls back to `process.cwd()`, so what this reads depends on From 3b46ef44a8a5194383a889cc1d4137ba0349fc3a Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Thu, 30 Jul 2026 13:29:19 +0200 Subject: [PATCH 03/28] fix(deps): require c12 >=3.2.0 for _configFile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit c12 introduced _configFile in 3.2.0, and load.ts uses it as the existence signal. Below that floor the field is always undefined, so every command threw NOT_INITIALIZED for a config that loads fine — hitting any consumer that resolved c12 via @prisma/config's 3.1.0 pin. --- package.json | 2 +- pnpm-lock.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 488550c..574cdc4 100644 --- a/package.json +++ b/package.json @@ -53,7 +53,7 @@ "prepublishOnly": "pnpm build" }, "dependencies": { - "c12": "^3.0.0", + "c12": "^3.2.0", "citty": "^0.2.2", "defu": "^6.1.4", "diff": "^9.0.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 3b2314b..c3a6de9 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -9,7 +9,7 @@ importers: .: dependencies: c12: - specifier: ^3.0.0 + specifier: ^3.2.0 version: 3.3.4 citty: specifier: ^0.2.2 From 144e1e1d25c3c0bb341f0c5d20cb756d32dd0ff5 Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Thu, 30 Jul 2026 13:29:40 +0200 Subject: [PATCH 04/28] test: note that the .config/.streamctl case guards a distinct regression --- test/resolve.test.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/resolve.test.ts b/test/resolve.test.ts index d229de1..ab4fdec 100644 --- a/test/resolve.test.ts +++ b/test/resolve.test.ts @@ -255,6 +255,12 @@ describe("resolveConfigFile", () => { // `.streamctl/config` ends in `/config` (no dot), survives the `.replace`, and // c12 probes `.config/.streamctl/config`. Removing it is intentional and is // called out in the release notes (P03-T03). + // + // This case guards a *different* regression than the other four: measured, it + // still passes if the root spelling is delegated to `loadConfig`, because the + // root spelling never probes this path. Only delegating the legacy spelling + // re-admits it. A refactor touching just the legacy path would leave the other + // four green and fail only here — do not dismiss that as a flake. await writeConfigDirFile(join(".streamctl", "config.ts")); expect(await resolveConfigFile(root, logger)).toBeNull(); From 83a6899ff3f6f273075805009c28c26efe850d87 Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Thu, 30 Jul 2026 13:34:04 +0200 Subject: [PATCH 05/28] feat(config): return the resolved location from the loader loadStreamctlConfig resolves the location first, calls c12 exactly once for the confirmed spelling, and returns { config, location } so writers can act on the file that was actually read. An uninitialized repo no longer reaches the loader at all. CONFIG_INVALID copy is now location-neutral. The probe and c12 must agree on which file won; they are compared as realpath'd, POSIX-normalized paths, since c12 emits pathe-normalized paths and exsolve resolves symlinks. --- src/config/load.ts | 70 +++++++++++++++++--- src/config/validate.ts | 2 +- test/load.test.ts | 141 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 201 insertions(+), 12 deletions(-) diff --git a/src/config/load.ts b/src/config/load.ts index 2527e80..d030b22 100644 --- a/src/config/load.ts +++ b/src/config/load.ts @@ -1,22 +1,69 @@ +import type { Logger } from "../logger"; +import type { ConfigFileLocation } from "./resolve"; import type { StreamctlConfig } from "./types"; +import { realpathSync } from "node:fs"; +import { sep } from "node:path"; import { loadConfig } from "c12"; import { StreamctlError } from "../errors"; +import { relativizeForDisplay } from "../paths"; +import { CONFIG_FILE, LEGACY_CONFIG_FILE, resolveConfigFile } from "./resolve"; import { validateStreamctlConfig } from "./validate"; +export interface LoadedStreamctlConfig { + config: StreamctlConfig; + location: ConfigFileLocation; +} + +/** + * Do two paths name the same underlying file? Not a string comparison, for three + * independent reasons: c12 takes its path helpers from pathe, which always emits + * forward slashes, while `location.abs` comes from `node:path` and is backslashed on + * Windows; exsolve may expand a Windows 8.3 short name; and `statSync` follows + * symlinks, so the probe reports the link while c12 may report its target. + * + * `realpathSync` throws if either path vanished between the probe and the load, which + * counts as a disagreement rather than an fs error to surface. + */ +function sameFile(a: string, b: string): boolean { + const norm = (path: string): string => realpathSync(path).split(sep).join("/"); + try { + return norm(a) === norm(b); + } catch { + return false; + } +} + /** - * Load and validate `.streamctl/config.ts` from `cwd`. + * Resolve, load and validate this repo's config: `streamctl.config.ts` at `cwd`, or a + * legacy `.streamctl/config.ts`. * - * - Missing config: throws `NOT_INITIALIZED`. + * - Missing config: throws `NOT_INITIALIZED`, without loading anything. * - Schema failure: throws `CONFIG_INVALID` (naming the offending field/path). * + * The location is resolved before c12 is involved, so `loadConfig` runs exactly once + * for a file already known to exist — and never at all for an uninitialized repo. + * * There is deliberately no framework detection in the CLI; detection is * payload-driven. */ -export async function loadStreamctlConfig(cwd: string): Promise { +export async function loadStreamctlConfig( + cwd: string, + opts?: { logger?: Logger }, +): Promise { + const location = await resolveConfigFile(cwd, opts?.logger); + if (location === null) { + throw new StreamctlError( + "NOT_INITIALIZED", + "No streamctl config found (streamctl.config.ts, or a legacy .streamctl/config.ts). Run `streamctl init` first.", + ); + } + + // The relative spelling, never `location.abs`: c12 builds jiti's base as + // `join(cwd, configFile)` (`dist/index.mjs:123`), which doubles an absolute path. const { config, _configFile } = await loadConfig>({ cwd, name: "streamctl", - configFile: ".streamctl/config", + configFile: location.source === "root" ? CONFIG_FILE : LEGACY_CONFIG_FILE, rcFile: false, globalRc: false, packageJson: false, @@ -24,12 +71,19 @@ export async function loadStreamctlConfig(cwd: string): Promise envName: false, }); - if (!_configFile) { + // The probe and c12 must agree on which file won. Disagreement means c12 found + // something the probe did not — i.e. the `.config/` exclusion has broken — so it is a + // broken installation or a c12 behavior change, not a user error. + if (_configFile === undefined || !sameFile(_configFile, location.abs)) { throw new StreamctlError( - "NOT_INITIALIZED", - "No .streamctl/config.ts found in this repo. Run `streamctl init` first.", + "CONFIG_INVALID", + `Resolved ${location.rel}, but c12 loaded a different file. This is a streamctl or c12 bug, not a problem with your config.`, + { + path: location.rel, + loaded: _configFile === undefined ? null : relativizeForDisplay(cwd, _configFile), + }, ); } - return validateStreamctlConfig(config); + return { config: validateStreamctlConfig(config), location }; } diff --git a/src/config/validate.ts b/src/config/validate.ts index a012522..e24777d 100644 --- a/src/config/validate.ts +++ b/src/config/validate.ts @@ -72,7 +72,7 @@ function collectStage1(input: unknown): { issues: ConfigIssue[]; rest: Record 0) { const summary = issues.map(issue => `${issue.path}: ${issue.message}`).join("; "); - throw new StreamctlError("CONFIG_INVALID", `Invalid .streamctl/config.ts: ${summary}`, { issues }); + throw new StreamctlError("CONFIG_INVALID", `Invalid streamctl config: ${summary}`, { issues }); } } diff --git a/test/load.test.ts b/test/load.test.ts index 91bacb6..7752639 100644 --- a/test/load.test.ts +++ b/test/load.test.ts @@ -1,14 +1,42 @@ +import { mkdtempSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { mkdir, mkdtemp, rm, symlink, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; -import { describe, expect, it } from "vitest"; +import { loadConfig } from "c12"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { loadStreamctlConfig } from "../src/config/load"; import { StreamctlError } from "../src/errors"; const fixtures = join(dirname(fileURLToPath(import.meta.url)), "fixtures"); +/** Can this platform/user create symlinks at all? Windows often can't. */ +function symlinksSupported(): boolean { + const dir = mkdtempSync(join(tmpdir(), "streamctl-symprobe-")); + try { + writeFileSync(join(dir, "t"), "x"); + symlinkSync(join(dir, "t"), join(dir, "l")); + return true; + } catch { + return false; + } finally { + rmSync(dir, { recursive: true, force: true }); + } +} + +const canSymlink = symlinksSupported(); + +const VALID_CONFIG = `export default { + package: "@acme/payload", + base: "nuxt-app", + version: "1.2.3", + profile: "nuxt-4", +}; +`; + describe("loadStreamctlConfig", () => { it("loads and validates a valid config", async () => { - const config = await loadStreamctlConfig(join(fixtures, "valid")); + const { config } = await loadStreamctlConfig(join(fixtures, "valid")); expect(config.base).toBe("nuxt-app"); expect(config.version).toBe("1.2.3"); expect(config.profile).toBe("nuxt-4"); @@ -19,6 +47,9 @@ describe("loadStreamctlConfig", () => { const error = await loadStreamctlConfig(join(fixtures, "uninitialized")).catch((e: unknown) => e); expect(error).toBeInstanceOf(StreamctlError); expect((error as StreamctlError).code).toBe("NOT_INITIALIZED"); + // Names the default location and the legacy one, since either is accepted. + expect((error as StreamctlError).message).toContain("streamctl.config.ts"); + expect((error as StreamctlError).message).toContain(".streamctl/config.ts"); }); it("throws CONFIG_INVALID naming `version` on bad semver", async () => { @@ -31,7 +62,7 @@ describe("loadStreamctlConfig", () => { it("lets an unknown top-level key through stage 1", async () => { // Unknown top-level keys are payload knobs. Loading has no merged chain to // check them against, so the strict pass lives in stage 2 (validateConfigKeys). - const config = await loadStreamctlConfig(join(fixtures, "unknown-key")); + const { config } = await loadStreamctlConfig(join(fixtures, "unknown-key")); expect(config.base).toBe("nuxt-app"); expect((config as Record).foo).toBe(true); }); @@ -41,4 +72,108 @@ describe("loadStreamctlConfig", () => { expect((error as StreamctlError).code).toBe("CONFIG_INVALID"); expect((error as StreamctlError).message).toContain("versionSyncExclude"); }); + + describe("location", () => { + it("reports the legacy location for a legacy fixture", async () => { + const cwd = join(fixtures, "valid"); + const { location } = await loadStreamctlConfig(cwd); + + expect(location.source).toBe("legacy"); + expect(location.rel).toBe(".streamctl/config.ts"); + expect(location.abs).toBe(join(cwd, ".streamctl", "config.ts")); + }); + + it("`location.abs` is the file c12 resolved", async () => { + const cwd = join(fixtures, "valid"); + const { location } = await loadStreamctlConfig(cwd); + + const { _configFile } = await loadConfig({ + cwd, + name: "streamctl", + configFile: ".streamctl/config", + rcFile: false, + globalRc: false, + packageJson: false, + dotenv: false, + envName: false, + }); + + // Compared as realpath'd forms, which is what the loader's own invariant means by + // agreement: c12 emits pathe-normalized (forward-slashed) paths and may expand a + // Windows 8.3 short name, so the raw strings can differ for the same file. + expect(realpathSync(location.abs)).toBe(realpathSync(_configFile ?? "")); + }); + }); + + describe("root location", () => { + let root: string; + + beforeEach(async () => { + root = realpathSync(await mkdtemp(join(tmpdir(), "streamctl-load-"))); + }); + + afterEach(async () => { + await rm(root, { recursive: true, force: true }); + }); + + it("loads a root config and reports it", async () => { + await writeFile(join(root, "streamctl.config.ts"), VALID_CONFIG); + + const { config, location } = await loadStreamctlConfig(root); + + expect(config.base).toBe("nuxt-app"); + expect(location.source).toBe("root"); + expect(location.rel).toBe("streamctl.config.ts"); + }); + + it("loads the root config and warns when both locations exist", async () => { + const warnings: string[] = []; + await writeFile(join(root, "streamctl.config.ts"), VALID_CONFIG); + await mkdir(join(root, ".streamctl"), { recursive: true }); + await writeFile(join(root, ".streamctl", "config.ts"), VALID_CONFIG.replace("nuxt-app", "legacy-base")); + + const { config, location } = await loadStreamctlConfig(root, { + logger: { warn: message => warnings.push(message) }, + }); + + expect(location.source).toBe("root"); + // The root config's `base`, proving the legacy file was not the one loaded. + expect(config.base).toBe("nuxt-app"); + // The ambiguity warning reaches the caller's logger through the loader. + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain("streamctl.config.ts"); + expect(warnings[0]).toContain(".streamctl/config.ts"); + }); + + it.skipIf(!canSymlink)("accepts a config that is a symlink", async () => { + // `statSync` follows the link, so the probe reports the link path while c12 may + // realpath to the target. The loader's invariant compares realpath'd forms, so + // this layout keeps working — a string comparison would fail it as CONFIG_INVALID, + // which would be a regression against today's behavior. + const shared = join(root, "shared"); + await mkdir(shared, { recursive: true }); + await writeFile(join(shared, "streamctl.ts"), VALID_CONFIG); + await symlink(join(shared, "streamctl.ts"), join(root, "streamctl.config.ts")); + + const { config, location } = await loadStreamctlConfig(root); + + expect(config.base).toBe("nuxt-app"); + expect(location.rel).toBe("streamctl.config.ts"); + expect(location.source).toBe("root"); + }); + + it.skipIf(!canSymlink)("accepts a legacy config that is a symlink", async () => { + const shared = join(root, "shared"); + await mkdir(join(root, ".streamctl"), { recursive: true }); + await mkdir(shared, { recursive: true }); + await writeFile(join(shared, "streamctl.ts"), VALID_CONFIG); + await symlink(join(shared, "streamctl.ts"), join(root, ".streamctl", "config.ts")); + + const { config, location } = await loadStreamctlConfig(root); + + expect(config.base).toBe("nuxt-app"); + expect(location.rel).toBe(".streamctl/config.ts"); + expect(location.source).toBe("legacy"); + }); + }); }); From ea20bdcfcb674b637f9f1e004d6190c10dde4918 Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Thu, 30 Jul 2026 13:35:57 +0200 Subject: [PATCH 06/28] test: close the post-hoc-rejection blind spot in the .config/ exclusion A regression that loaded speculatively and rejected .config/ hits afterwards would still return null while having executed the file. The sentinel body catches it. Also records where the /index divergence is decided, so deleting that test reads as a spec change. --- test/resolve.test.ts | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/test/resolve.test.ts b/test/resolve.test.ts index ab4fdec..7d5d97e 100644 --- a/test/resolve.test.ts +++ b/test/resolve.test.ts @@ -234,9 +234,17 @@ describe("resolveConfigFile", () => { // never resolved either, because c12 probes `.config/.streamctl/config` and never // `.config/streamctl`. Switching to the root spelling is what *would* have // started resolving it — verified against c12 3.3.4. This prevents that. - await writeConfigDirFile("streamctl.ts"); + // + // The side-effect body closes a blind spot the `null` assertion alone leaves: a + // regression that loads speculatively and *then* rejects `.config/` hits would + // still return `null` here, having already executed this file. That is the + // strictly-worse design `20_architecture.md` rejects — post-hoc rejection cannot + // undo an evaluation, because c12 loads as part of resolving. + const sentinel = join(root, "config-dir-evaluated"); + await writeConfigDirFile("streamctl.ts", sideEffectConfig(sentinel)); expect(await resolveConfigFile(root, logger)).toBeNull(); + expect(existsSync(sentinel)).toBe(false); expect(warnings).toEqual([]); }); @@ -288,11 +296,13 @@ describe("resolveConfigFile", () => { describe("directory-shaped config via c12's /index suffix", () => { it("does not resolve streamctl.config/index.ts", async () => { - // A real, intentional divergence: c12 accepts this form via - // `suffixes: ["", "/index"]` (`dist/index.mjs:338`) — verified, it loads — and the - // probe deliberately does not mirror it, because a directory-shaped config is - // outside the two-locations promise. Distinct from the `streamctl.config.ts`-as-a - // -directory case above, where the candidate itself is the directory. + // A real, intentional divergence, recorded as deliberate in `20_architecture.md` + // ("Resolution strategy" — the one property given up by probe-then-load): c12 + // accepts this form via `suffixes: ["", "/index"]` (`dist/index.mjs:338`) — + // verified, it loads — and the probe does not mirror it, because a directory-shaped + // config is outside the two-locations promise. Deleting this test is a spec change, + // not a cleanup. Distinct from the `streamctl.config.ts`-as-a-directory case above, + // where the candidate itself is the directory. await mkdir(join(root, "streamctl.config")); await writeFile(join(root, "streamctl.config", "index.ts"), "export default {}\n"); From 4c3435d34453ba77c003cf6ad6bb49e14f9ed543 Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Thu, 30 Jul 2026 13:41:06 +0200 Subject: [PATCH 07/28] feat: read the config from either location in every command The four loader call sites take { config, location } and pass their reporter as the logger, so the both-present warning reaches stderr and stays out of the --json envelope on stdout. NOT_INITIALIZED now names both accepted locations. --- src/commands/check.ts | 2 +- src/commands/status.ts | 2 +- src/commands/sync.ts | 2 +- src/engine/upgrade.ts | 4 +- test/__snapshots__/cli.test.ts.snap | 2 +- test/ambiguity.command.test.ts | 95 +++++++++++++++++++++++++++++ test/helpers/streams.ts | 24 ++++++++ 7 files changed, 126 insertions(+), 5 deletions(-) create mode 100644 test/ambiguity.command.test.ts create mode 100644 test/helpers/streams.ts diff --git a/src/commands/check.ts b/src/commands/check.ts index 8584db4..ead45be 100644 --- a/src/commands/check.ts +++ b/src/commands/check.ts @@ -34,7 +34,7 @@ export const checkCommand = defineCommand({ ); } const cwd = process.cwd(); - const config = await loadStreamctlConfig(cwd); + const { config } = await loadStreamctlConfig(cwd, { logger: reporter }); const payload = await resolvePayload(cwd, config.package, config.version); return runCheck(cwd, payload, config, failOn, { logger: reporter }); }); diff --git a/src/commands/status.ts b/src/commands/status.ts index 50dc195..e4639e1 100644 --- a/src/commands/status.ts +++ b/src/commands/status.ts @@ -22,7 +22,7 @@ export const statusCommand = defineCommand({ // so a successful run always exits 0. Only a config-load/payload error takes exit-1. return executeCommand("status", options.json, async (reporter) => { const cwd = process.cwd(); - const config = await loadStreamctlConfig(cwd); + const { config } = await loadStreamctlConfig(cwd, { logger: reporter }); const payload = await resolvePayload(cwd, config.package, config.version); return runStatus(cwd, payload, config, { cliVersion: readCliVersion(), diff --git a/src/commands/sync.ts b/src/commands/sync.ts index 2cda366..85c5444 100644 --- a/src/commands/sync.ts +++ b/src/commands/sync.ts @@ -39,7 +39,7 @@ export const syncCommand = defineCommand({ return executeCommand("sync", options.json, async (reporter) => { rejectEmptyFlags(args); const cwd = process.cwd(); - const loaded = await loadStreamctlConfig(cwd); + const { config: loaded } = await loadStreamctlConfig(cwd, { logger: reporter }); const config = options.versionSync ? loaded : { ...loaded, versionSync: false }; const payload = await resolvePayload(cwd, config.package, config.version); await warnProfileMismatch(cwd, payload, config.profile, reporter); diff --git a/src/engine/upgrade.ts b/src/engine/upgrade.ts index 4625589..f7479d9 100644 --- a/src/engine/upgrade.ts +++ b/src/engine/upgrade.ts @@ -331,7 +331,9 @@ async function previewSync(opts: RunUpgradeOptions, config: StreamctlConfig, onP export async function runUpgrade(opts: RunUpgradeOptions): Promise { const { cwd, dryRun } = opts; - const config = await loadStreamctlConfig(cwd); + // Phase 2 also destructures `location` here, for the snapshot and the pin bump; both + // still use the hardcoded literal. + const { config } = await loadStreamctlConfig(cwd, { logger: opts.logger }); const fromVersion = config.version; // A payload pinned via a local override (a package-manager `overrides` entry diff --git a/test/__snapshots__/cli.test.ts.snap b/test/__snapshots__/cli.test.ts.snap index 4197ef6..760d2d0 100644 --- a/test/__snapshots__/cli.test.ts.snap +++ b/test/__snapshots__/cli.test.ts.snap @@ -5,7 +5,7 @@ exports[`check --json on an uninitialized repo > emits a well-formed JSON error "command": "check", "error": { "code": "NOT_INITIALIZED", - "message": "No .streamctl/config.ts found in this repo. Run \`streamctl init\` first.", + "message": "No streamctl config found (streamctl.config.ts, or a legacy .streamctl/config.ts). Run \`streamctl init\` first.", }, "exitCode": 1, "ok": false, diff --git a/test/ambiguity.command.test.ts b/test/ambiguity.command.test.ts new file mode 100644 index 0000000..77a7dab --- /dev/null +++ b/test/ambiguity.command.test.ts @@ -0,0 +1,95 @@ +import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { statusCommand } from "../src/commands/status"; +import { captureStderr } from "./helpers/streams"; + +type StatusArgs = Parameters>[0]; + +/** The root config pins this; the legacy config pins something else on purpose. */ +const VERSION = "9.9.9"; +const LEGACY_VERSION = "1.1.1"; + +const TEMPLATES: Record = { + "manifest.json": JSON.stringify({ schemaVersion: 2, presets: ["base"], profiles: [], defaultBase: "base" }), + "base/preset.json": JSON.stringify({ + name: "base", + files: [{ path: ".editorconfig", strategy: "full", source: "base/editorconfig" }], + }), + "base/editorconfig": "root = true\n", +}; + +function config(version: string): string { + return `export default { package: "@acme/payload", base: "base", version: "${version}", profile: "nuxt-4" };\n`; +} + +let repo: string; +const previousExitCode = process.exitCode; +const stdout: string[] = []; + +beforeEach(async () => { + repo = await mkdtemp(join(tmpdir(), "streamctl-ambiguity-")); + const pkg = join(repo, "node_modules", "@acme", "payload"); + await mkdir(join(pkg, "presets", "base"), { recursive: true }); + await writeFile(join(pkg, "package.json"), JSON.stringify({ name: "@acme/payload", version: VERSION })); + for (const [source, content] of Object.entries(TEMPLATES)) { + await writeFile(join(pkg, "presets", source), content); + } + await writeFile(join(repo, ".editorconfig"), "root = true\n"); + + // Both locations, which is the case under test. + await writeFile(join(repo, "streamctl.config.ts"), config(VERSION)); + await mkdir(join(repo, ".streamctl"), { recursive: true }); + await writeFile(join(repo, ".streamctl", "config.ts"), config(LEGACY_VERSION)); + + stdout.length = 0; + vi.spyOn(process, "cwd").mockReturnValue(repo); + vi.spyOn(process.stdout, "write").mockImplementation((chunk: unknown) => { + stdout.push(String(chunk)); + return true; + }); + vi.spyOn(process.stderr, "write").mockImplementation(() => true); +}); + +afterEach(async () => { + process.exitCode = previousExitCode; + vi.restoreAllMocks(); + await rm(repo, { recursive: true, force: true }); +}); + +describe("both config locations present", () => { + it("warns on stderr and keeps the --json envelope clean", async () => { + const stderr = captureStderr(); + + await statusCommand.run?.({ args: { json: true } } as unknown as StatusArgs); + + expect(process.exitCode).toBe(0); + + // stdout is the envelope and nothing else: it must still parse, and must not carry + // the warning text. A `console.warn` in the resolver would break both. + const envelope = JSON.parse(stdout.join("")) as { ok: boolean; data: { payload: { pinned: string } } }; + expect(envelope.ok).toBe(true); + expect(stdout.join("")).not.toContain("streamctl:"); + + // Exactly one warning, naming both paths. Substrings only — Q1's wording is open. + const lines = stderr.join("").split("\n").filter(line => line.length > 0); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain("streamctl.config.ts"); + expect(lines[0]).toContain(".streamctl/config.ts"); + + // The root config is the one that was read: it pins the installed version, while the + // legacy file pins something else. + expect(envelope.data.payload.pinned).toBe(VERSION); + }); + + it("warns once on a non-json run too", async () => { + const stderr = captureStderr(); + + await statusCommand.run?.({ args: {} } as unknown as StatusArgs); + + expect(process.exitCode).toBe(0); + const lines = stderr.join("").split("\n").filter(line => line.length > 0); + expect(lines).toHaveLength(1); + }); +}); diff --git a/test/helpers/streams.ts b/test/helpers/streams.ts new file mode 100644 index 0000000..102097c --- /dev/null +++ b/test/helpers/streams.ts @@ -0,0 +1,24 @@ +import { vi } from "vitest"; + +/** + * Capture a stream instead of discarding it. Command test files stub both stdout and + * stderr to `() => true` in `beforeEach`; call one of these from a test that needs to + * read the writes back. The later `vi.spyOn` replaces the earlier stub, and + * `vi.restoreAllMocks()` in `afterEach` undoes both. + */ +function capture(stream: "stdout" | "stderr"): string[] { + const writes: string[] = []; + vi.spyOn(process[stream], "write").mockImplementation((chunk: unknown) => { + writes.push(String(chunk)); + return true; + }); + return writes; +} + +export function captureStdout(): string[] { + return capture("stdout"); +} + +export function captureStderr(): string[] { + return capture("stderr"); +} From b04490bdab842e3a6c66b91a9d14184693a1d085 Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Thu, 30 Jul 2026 13:48:47 +0200 Subject: [PATCH 08/28] test: correct the console.warn attribution in the ambiguity test --- test/ambiguity.command.test.ts | 9 ++++++++- test/helpers/streams.ts | 6 ++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/test/ambiguity.command.test.ts b/test/ambiguity.command.test.ts index 77a7dab..603fc60 100644 --- a/test/ambiguity.command.test.ts +++ b/test/ambiguity.command.test.ts @@ -67,12 +67,19 @@ describe("both config locations present", () => { expect(process.exitCode).toBe(0); // stdout is the envelope and nothing else: it must still parse, and must not carry - // the warning text. A `console.warn` in the resolver would break both. + // the warning text. const envelope = JSON.parse(stdout.join("")) as { ok: boolean; data: { payload: { pinned: string } } }; expect(envelope.ok).toBe(true); expect(stdout.join("")).not.toContain("streamctl:"); // Exactly one warning, naming both paths. Substrings only — Q1's wording is open. + // + // This length assertion, not the stdout ones above, is what a `console.warn` in the + // resolver would break: `console.warn` goes to stderr, so stdout stays clean either + // way. And it only catches it because vitest intercepts `console`, bypassing the + // `process.stderr.write` spy — in production `console.warn` does reach stderr. So + // this is a harness artifact, not evidence that `console.*` in the resolver is + // caught. The `Logger` seam is enforced by review, not by this test. const lines = stderr.join("").split("\n").filter(line => line.length > 0); expect(lines).toHaveLength(1); expect(lines[0]).toContain("streamctl.config.ts"); diff --git a/test/helpers/streams.ts b/test/helpers/streams.ts index 102097c..b110839 100644 --- a/test/helpers/streams.ts +++ b/test/helpers/streams.ts @@ -5,6 +5,12 @@ import { vi } from "vitest"; * stderr to `() => true` in `beforeEach`; call one of these from a test that needs to * read the writes back. The later `vi.spyOn` replaces the earlier stub, and * `vi.restoreAllMocks()` in `afterEach` undoes both. + * + * `test/init.command.test.ts:12-20` duplicates `captureStdout` rather than importing it, + * deliberately: that file is a regression witness for this feature and has to stay + * untouched across the whole branch, so even adding this note to it would break the + * property it exists to prove. The duplication must survive until the witness is + * retired; `P03-T01` is the earliest point it can be revisited. */ function capture(stream: "stdout" | "stderr"): string[] { const writes: string[] = []; From 0064ef4f2a8fec549d86a808db9fb9dd22151a98 Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Thu, 30 Jul 2026 16:38:30 +0200 Subject: [PATCH 09/28] feat(upgrade): snapshot and bump the config that was actually loaded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rollback snapshot and the version pin bump both take the resolved location, so a legacy repo's transaction covers .streamctl/config.ts instead of a root path that never existed — which restoreFile would have reported as a successful no-op restore. --- src/engine/upgrade.ts | 44 ++++++++++---------- test/upgrade.test.ts | 93 +++++++++++++++++++++++++++++++++++++------ 2 files changed, 103 insertions(+), 34 deletions(-) diff --git a/src/engine/upgrade.ts b/src/engine/upgrade.ts index f7479d9..fae8f9a 100644 --- a/src/engine/upgrade.ts +++ b/src/engine/upgrade.ts @@ -1,3 +1,4 @@ +import type { ConfigFileLocation } from "../config/resolve"; import type { StreamctlConfig } from "../config/types"; import type { Logger } from "../logger"; import type { ConfigKeyType } from "../manifest/schema"; @@ -97,9 +98,9 @@ interface FileSnapshot { } /** - * Move the `.streamctl/config.ts > version` pin in place. Line-anchored so a suffix - * key (`myversion:`), a `versionSync:` sibling, or a commented-out pin cannot be - * mistaken for the real one. + * Move the resolved config's `version` pin in place, at whichever location the run + * loaded it from. Line-anchored so a suffix key (`myversion:`), a `versionSync:` + * sibling, or a commented-out pin cannot be mistaken for the real one. * * The pin is chosen by indentation, not document order: the streamctl pin is * top-level and therefore the shallowest `version:` line, while a payload knob like @@ -110,11 +111,10 @@ interface FileSnapshot { * * Matched rather than parsed; the config is TS and full TS parsing is out of scope. */ -async function bumpConfigVersion(cwd: string, toVersion: string): Promise { - const abs = join(cwd, ".streamctl", "config.ts"); - const raw = await readFileOrNull(abs); +async function bumpConfigVersion(location: ConfigFileLocation, toVersion: string): Promise { + const raw = await readFileOrNull(location.abs); if (raw === null) { - throw new StreamctlError("CONFIG_INVALID", "`.streamctl/config.ts` not found.", { path: ".streamctl/config.ts" }); + throw new StreamctlError("CONFIG_INVALID", `\`${location.rel}\` not found.`, { path: location.rel }); } // Horizontal whitespace only in the indent capture. `\s*` would span newlines: // under `/m` the `^` also asserts at a blank line, and a greedy `\s*` then eats the @@ -128,8 +128,8 @@ async function bumpConfigVersion(cwd: string, toVersion: string): Promise if (matches.length === 0) { throw new StreamctlError( "CONFIG_INVALID", - "Could not find a `version: \"…\"` pin on its own line in .streamctl/config.ts to bump.", - { path: ".streamctl/config.ts" }, + `Could not find a \`version: "…"\` pin on its own line in ${location.rel} to bump.`, + { path: location.rel }, ); } @@ -140,8 +140,8 @@ async function bumpConfigVersion(cwd: string, toVersion: string): Promise if (outermost.length > 1) { throw new StreamctlError( "CONFIG_INVALID", - `Ambiguous version pin in .streamctl/config.ts: ${outermost.length} \`version:\` keys share the outermost indentation, so the streamctl pin cannot be identified. Leave exactly one \`version:\` at the top level of the exported config.`, - { path: ".streamctl/config.ts" }, + `Ambiguous version pin in ${location.rel}: ${outermost.length} \`version:\` keys share the outermost indentation, so the streamctl pin cannot be identified. Leave exactly one \`version:\` at the top level of the exported config.`, + { path: location.rel }, ); } @@ -149,7 +149,7 @@ async function bumpConfigVersion(cwd: string, toVersion: string): Promise // file-wide replace did. The callback form keeps `$`-patterns in `toVersion` literal. const matched = raw.slice(target.index, target.index + target[0].length); const bumped = matched.replace(/(["'])[^"']*(["'])$/, (_match: string, open: string, close: string) => `${open}${toVersion}${close}`); - await atomicWrite(abs, raw.slice(0, target.index) + bumped + raw.slice(target.index + target[0].length)); + await atomicWrite(location.abs, raw.slice(0, target.index) + bumped + raw.slice(target.index + target[0].length)); } /** @@ -318,8 +318,8 @@ async function previewSync(opts: RunUpgradeOptions, config: StreamctlConfig, onP * The only command that moves the pinned version forward, and it does so * transactionally: either it fully applies or it restores the exact pre-upgrade tree. * - * Three files are snapshotted, since a failed run could leave them inconsistent: - * `.streamctl/config.ts`, `package.json`, and the detected PM's lockfile. The pin is + * Three files are snapshotted, since a failed run could leave them inconsistent: the + * resolved config (root or legacy), `package.json`, and the detected PM's lockfile. The pin is * validated and written before install so a failed install rolls back cleanly, and * `runSync` writes nothing until its batch is clean, which doubles as the preflight. * @@ -331,9 +331,9 @@ async function previewSync(opts: RunUpgradeOptions, config: StreamctlConfig, onP export async function runUpgrade(opts: RunUpgradeOptions): Promise { const { cwd, dryRun } = opts; - // Phase 2 also destructures `location` here, for the snapshot and the pin bump; both - // still use the hardcoded literal. - const { config } = await loadStreamctlConfig(cwd, { logger: opts.logger }); + // `location` feeds both config touch points below — the rollback snapshot and the pin + // bump — so the run can only ever write the file it read. + const { config, location } = await loadStreamctlConfig(cwd, { logger: opts.logger }); const fromVersion = config.version; // A payload pinned via a local override (a package-manager `overrides` entry @@ -428,9 +428,11 @@ export async function runUpgrade(opts: RunUpgradeOptions): Promise { } } -const writeConfig = (content: string): Promise => writeFile(join(repo, ".streamctl", "config.ts"), content); +const writeConfig = (content: string): Promise => writeFile(join(repo, "streamctl.config.ts"), content); /** A clean initialized repo pinned to FROM (config.ts + devDeps at FROM). */ async function makeRepo(): Promise { @@ -63,7 +63,6 @@ async function makeRepo(): Promise { join(repo, "package.json"), `${JSON.stringify({ name: "app", packageManager: "npm@10.0.0", devDependencies: { "@acme/payload": FROM, "@sidebase/streamctl": FROM } }, null, 2)}\n`, ); - await mkdir(join(repo, ".streamctl"), { recursive: true }); await writeConfig(`export default {\n package: "@acme/payload",\n base: "base",\n version: "${FROM}",\n profile: "nuxt-4",\n};\n`); } @@ -84,7 +83,7 @@ function baseOpts(overrides: Partial = {}): RunUpgradeOptions async function readPkg(): Promise<{ devDependencies: Record }> { return JSON.parse(await readFile(join(repo, "package.json"), "utf8")); } -const readConfig = (): Promise => readFile(join(repo, ".streamctl", "config.ts"), "utf8"); +const readConfig = (): Promise => readFile(join(repo, "streamctl.config.ts"), "utf8"); /** sha256 of a repo-relative file, `null` when absent. The byte-identity oracle for rollback tests. */ async function hashFile(rel: string): Promise { @@ -95,7 +94,7 @@ async function hashFile(rel: string): Promise { /** Snapshot the three transactional files (config.ts, package.json, lockfile) as hashes. */ async function hashSnapshot(lockfile = "package-lock.json"): Promise> { return { - config: await hashFile(".streamctl/config.ts"), + config: await hashFile("streamctl.config.ts"), pkg: await hashFile("package.json"), lock: await hashFile(lockfile), }; @@ -226,7 +225,7 @@ describe("runUpgrade", () => { }); it("NOT_INITIALIZED on a config-less repo", async () => { - await rm(join(repo, ".streamctl"), { recursive: true, force: true }); + await rm(join(repo, "streamctl.config.ts"), { force: true }); const error = await runUpgrade(baseOpts()).catch((e: unknown) => e); expect(error).toBeInstanceOf(StreamctlError); expect((error as StreamctlError).code).toBe("NOT_INITIALIZED"); @@ -288,7 +287,7 @@ describe("runUpgrade", () => { // never existed, so the failed install's damage to the REAL lockfile would // survive a rollback that reported success. const pkgDir = join(repo, "packages", "app"); - await mkdir(join(pkgDir, ".streamctl"), { recursive: true }); + await mkdir(pkgDir, { recursive: true }); await writeFile(join(repo, ".git"), "gitdir: ../elsewhere\n"); const rootLock = join(repo, "pnpm-lock.yaml"); await writeFile(rootLock, "lockfileVersion: 9\n# pinned to 1.0.0\n"); @@ -296,7 +295,7 @@ describe("runUpgrade", () => { join(pkgDir, "package.json"), `${JSON.stringify({ name: "app", packageManager: "pnpm@10.28.1", devDependencies: { "@acme/payload": FROM, "@sidebase/streamctl": FROM } }, null, 2)}\n`, ); - await writeFile(join(pkgDir, ".streamctl", "config.ts"), `export default {\n package: "@acme/payload",\n base: "base",\n version: "${FROM}",\n profile: "nuxt-4",\n};\n`); + await writeFile(join(pkgDir, "streamctl.config.ts"), `export default {\n package: "@acme/payload",\n base: "base",\n version: "${FROM}",\n profile: "nuxt-4",\n};\n`); const before = await readFile(rootLock, "utf8"); const install = vi.fn(async () => { @@ -317,14 +316,14 @@ describe("runUpgrade", () => { // `git checkout HEAD -- ` command. Git pathspecs resolve against cwd, so // a `../`-style label stays copy-pasteable from the package dir. const pkgDir = join(repo, "packages", "app"); - await mkdir(join(pkgDir, ".streamctl"), { recursive: true }); + await mkdir(pkgDir, { recursive: true }); await writeFile(join(repo, ".git"), "gitdir: ../elsewhere\n"); await writeFile(join(repo, "pnpm-lock.yaml"), "lockfileVersion: 9\n"); await writeFile( join(pkgDir, "package.json"), `${JSON.stringify({ name: "app", packageManager: "pnpm@10.28.1", devDependencies: { "@acme/payload": FROM, "@sidebase/streamctl": FROM } }, null, 2)}\n`, ); - await writeFile(join(pkgDir, ".streamctl", "config.ts"), `export default {\n package: "@acme/payload",\n base: "base",\n version: "${FROM}",\n profile: "nuxt-4",\n};\n`); + await writeFile(join(pkgDir, "streamctl.config.ts"), `export default {\n package: "@acme/payload",\n base: "base",\n version: "${FROM}",\n profile: "nuxt-4",\n};\n`); const install = vi.fn(async () => { // Dirty the lockfile, or the restore is a no-op and never reports a failure. @@ -349,13 +348,13 @@ describe("runUpgrade", () => { // to cwd. The install then writes the ROOT lockfile, which the snapshot could not // have predicted, so it has to be swept rather than restored. const pkgDir = join(repo, "packages", "app"); - await mkdir(join(pkgDir, ".streamctl"), { recursive: true }); + await mkdir(pkgDir, { recursive: true }); await writeFile(join(repo, ".git"), "gitdir: ../elsewhere\n"); await writeFile( join(pkgDir, "package.json"), `${JSON.stringify({ name: "app", packageManager: "pnpm@10.28.1", devDependencies: { "@acme/payload": FROM, "@sidebase/streamctl": FROM } }, null, 2)}\n`, ); - await writeFile(join(pkgDir, ".streamctl", "config.ts"), `export default {\n package: "@acme/payload",\n base: "base",\n version: "${FROM}",\n profile: "nuxt-4",\n};\n`); + await writeFile(join(pkgDir, "streamctl.config.ts"), `export default {\n package: "@acme/payload",\n base: "base",\n version: "${FROM}",\n profile: "nuxt-4",\n};\n`); const rootLock = join(repo, "pnpm-lock.yaml"); const install = vi.fn(async () => { @@ -459,7 +458,7 @@ describe("runUpgrade", () => { expect((error as StreamctlError).code).toBe("ROLLBACK_FAILED"); const details = (error as StreamctlError).details as { failed: string[]; recover: string; perFile: string[] }; - expect(details.failed).toEqual(expect.arrayContaining([".streamctl/config.ts", "package.json"])); + expect(details.failed).toEqual(expect.arrayContaining(["streamctl.config.ts", "package.json"])); expect(details.recover).toContain("git checkout HEAD --"); expect(details.perFile.some(line => line.includes("FAILED"))).toBe(true); }); @@ -946,7 +945,7 @@ describe("runUpgrade: interrupt signal handling", () => { expect(capturedSignal).toBe("SIGINT"); expect(abortCount).toBe(1); // second signal ignored - expect(capturedStatuses?.map(s => s.path).sort()).toEqual([".streamctl/config.ts", "package-lock.json", "package.json"]); + expect(capturedStatuses?.map(s => s.path).sort()).toEqual(["package-lock.json", "package.json", "streamctl.config.ts"]); expect(capturedStatuses?.every(s => s.ok)).toBe(true); expect(await hashSnapshot()).toEqual(before); expect(await readConfig()).toContain(`version: "${FROM}"`); @@ -960,3 +959,71 @@ describe("runUpgrade: interrupt signal handling", () => { expect(process.listenerCount("SIGTERM")).toBe(before.term); }); }); + +/** + * A legacy repo keeps its config at `.streamctl/config.ts`, and `upgrade` must act on + * that file rather than on the root path it would write today. Every assertion here is + * on file **content or hash**, never on reported restore status: `restoreFile` returns + * `{ action: "unchanged", ok: true }` for a path that never existed, so a status-based + * assertion passes in exactly the broken case these tests exist to catch. + */ +describe("runUpgrade: legacy config location", () => { + const legacyRel = ".streamctl/config.ts"; + const legacyConfig = `export default {\n package: "@acme/payload",\n base: "base",\n version: "${FROM}",\n profile: "nuxt-4",\n};\n`; + + /** Move this repo's config from the root to the legacy location. */ + async function useLegacyConfig(): Promise { + await rm(join(repo, "streamctl.config.ts"), { force: true }); + await mkdir(join(repo, ".streamctl"), { recursive: true }); + await writeFile(join(repo, legacyRel), legacyConfig); + } + + const readLegacyConfig = (): Promise => readFile(join(repo, legacyRel), "utf8"); + + it("moves the pin inside the legacy file", async () => { + await useLegacyConfig(); + + const result = await runUpgrade(baseOpts()); + + expect(result.toVersion).toBe(TO); + expect(await readLegacyConfig()).toContain(`version: "${TO}"`); + // The root path must not be created as a side effect of the bump. + expect(existsSync(join(repo, "streamctl.config.ts"))).toBe(false); + }); + + it("restores the legacy file byte-for-byte after a failed install", async () => { + await useLegacyConfig(); + const before = await hashFile(legacyRel); + const install = vi.fn(async () => { + throw new Error("install boom"); + }); + + const error = await runUpgrade(baseOpts({ install })).catch((e: unknown) => e); + + expect(error).toBeInstanceOf(StreamctlError); + // The hash is the oracle: the bump advanced the pin to TO, so an unrestored file + // hashes differently. A rollback that "succeeded" without writing fails here. + expect(await hashFile(legacyRel)).toBe(before); + expect(await readLegacyConfig()).toContain(`version: "${FROM}"`); + expect(await readLegacyConfig()).not.toContain(`version: "${TO}"`); + }); + + it("ROLLBACK_FAILED names the legacy path, not the root one", async () => { + await useLegacyConfig(); + const install = vi.fn(async () => { + throw new Error("install boom"); + }); + const restoreWrite = vi.fn(async () => { + throw new Error("disk full"); + }); + + const error = await runUpgrade(baseOpts({ install, restoreWrite })).catch((e: unknown) => e); + + expect((error as StreamctlError).code).toBe("ROLLBACK_FAILED"); + const details = (error as StreamctlError).details as { failed: string[]; recover: string }; + expect(details.failed).toContain(legacyRel); + expect(details.failed).not.toContain("streamctl.config.ts"); + // The label doubles as the git pathspec in the recovery command. + expect(details.recover).toContain(legacyRel); + }); +}); From 96322cfc87b31e16d3a71e5cb95fec7ae6e4fde0 Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Thu, 30 Jul 2026 16:45:40 +0200 Subject: [PATCH 10/28] test(upgrade): pin the CONFIG_INVALID path to the resolved config --- src/engine/upgrade.ts | 7 ++++--- test/upgrade.test.ts | 17 +++++++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/engine/upgrade.ts b/src/engine/upgrade.ts index fae8f9a..5b8a98e 100644 --- a/src/engine/upgrade.ts +++ b/src/engine/upgrade.ts @@ -319,9 +319,10 @@ async function previewSync(opts: RunUpgradeOptions, config: StreamctlConfig, onP * transactionally: either it fully applies or it restores the exact pre-upgrade tree. * * Three files are snapshotted, since a failed run could leave them inconsistent: the - * resolved config (root or legacy), `package.json`, and the detected PM's lockfile. The pin is - * validated and written before install so a failed install rolls back cleanly, and - * `runSync` writes nothing until its batch is clean, which doubles as the preflight. + * resolved config (root or legacy), `package.json`, and the detected PM's lockfile. + * The pin is validated and written before install so a failed install rolls back + * cleanly, and `runSync` writes nothing until its batch is clean, which doubles as the + * preflight. * * Any failure after the snapshot restores it byte-exactly and re-throws the original * error tagged `rolledBack: true`; only `node_modules` reflects the aborted install. diff --git a/test/upgrade.test.ts b/test/upgrade.test.ts index bfc6aaf..6369579 100644 --- a/test/upgrade.test.ts +++ b/test/upgrade.test.ts @@ -1008,6 +1008,23 @@ describe("runUpgrade: legacy config location", () => { expect(await readLegacyConfig()).not.toContain(`version: "${TO}"`); }); + it("CONFIG_INVALID details name the legacy path when no pin is bumpable", async () => { + await useLegacyConfig(); + // Single-line object: the pin regex is line-anchored, so `version:` mid-line is not + // a bumpable pin. The error then has to name the file the user actually has. + await writeFile(join(repo, legacyRel), `export default { package: "@acme/payload", base: "base", version: "${FROM}", profile: "nuxt-4" };\n`); + + const error = await runUpgrade(baseOpts({ install: vi.fn(async () => {}) })).catch((e: unknown) => e); + + expect((error as StreamctlError).code).toBe("CONFIG_INVALID"); + const details = (error as StreamctlError).details as { path: string }; + expect(details.path).toBe(legacyRel); + // The negative is the point: a regression to the literal would send a legacy-repo + // user to a root file that does not exist. + expect(details.path).not.toBe("streamctl.config.ts"); + expect((error as StreamctlError).message).toContain(legacyRel); + }); + it("ROLLBACK_FAILED names the legacy path, not the root one", async () => { await useLegacyConfig(); const install = vi.fn(async () => { From 700924f8fcb21ba1544a39f9fef42a45ee4c8cf7 Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Thu, 30 Jul 2026 16:45:47 +0200 Subject: [PATCH 11/28] feat(manifest): reserve the streamctl config paths from payload management MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A payload managing the config would have sync overwrite the file the same run just read, and fight upgrade's rollback snapshot. Reservations compare a posix-normalized path, so ./ and .// spellings of the same file cannot slip past — which also closes that hole in the existing package.json reservation. --- src/manifest/schema.ts | 31 ++++++++++++++++++++++- test/manifest-schema.test.ts | 48 ++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/src/manifest/schema.ts b/src/manifest/schema.ts index 9ec8ffc..0512457 100644 --- a/src/manifest/schema.ts +++ b/src/manifest/schema.ts @@ -1,9 +1,18 @@ import type { ZodError } from "zod"; import type { ConfigIssue } from "../config/validate"; +import { posix } from "node:path"; import { z } from "zod"; import { RECONCILABLE_KEY_PATTERN } from "../config/types"; import { isStructuredPath } from "../paths"; +/** + * Config paths a payload may not manage. Spelled out here rather than imported from + * `config/resolve`, so this separately published entry point stays independent of the + * loader — the strings are stable and the coupling would buy nothing. + */ +const CONFIG_STEM = "streamctl.config"; +const LEGACY_CONFIG_DIR = ".streamctl"; + /** A payload declaring any other `schemaVersion` maps to `SCHEMA_UNSUPPORTED`. Manifest and package versions are otherwise independent. */ export const SUPPORTED_SCHEMA_VERSION = 2; @@ -70,11 +79,31 @@ export const managedFileSchema = z.strictObject({ if (file.path.split("/").includes("..")) { ctx.addIssue({ code: "custom", path: ["path"], message: "must not contain \"..\" segments" }); } + // Reservations below compare against the normalized path, because `./x`, `.//x` and + // `././x` all reach disk as `join(cwd, path)` — the same file a bare `x` names, so an + // equality or prefix test on the raw value is trivially bypassed. + // + // `posix.normalize`, never the platform `normalize`: the latter emits backslashes on + // Windows, which the check three lines above rejects. It also leaves a leading `..` + // in place, so the `..` check keeps firing on the raw path; a doubly-bad path simply + // collects two issues, which is why this is not an early return. + const reserved = posix.normalize(file.path); // `package.json` is owned by the version reconcile (writes it last from a pre-write // snapshot); managing it as a file too would let the reconcile silently clobber that write. - if (file.path === "package.json") { + if (reserved === "package.json") { ctx.addIssue({ code: "custom", path: ["path"], message: "package.json is reconciled via versionSync, not a managed file" }); } + // The config is written by `init` and rewritten by `upgrade`'s pin bump, which also + // snapshots it for rollback; managing it as a file too would have `sync` overwrite the + // config the same run just read. Matched by stem so every extension is covered without + // this module importing the loader's extension list. Root-level only: the reservation + // is for the invocation-directory config, so `nested/streamctl.config.ts` is fine. + if (!reserved.includes("/") && reserved.startsWith(`${CONFIG_STEM}.`)) { + ctx.addIssue({ code: "custom", path: ["path"], message: `${CONFIG_STEM}.* is written by \`streamctl init\` and rewritten by \`streamctl upgrade\`, not a managed file` }); + } + if (reserved.startsWith(`${LEGACY_CONFIG_DIR}/`)) { + ctx.addIssue({ code: "custom", path: ["path"], message: `${LEGACY_CONFIG_DIR}/ holds the legacy streamctl config, which \`streamctl upgrade\` rewrites; it cannot hold managed files` }); + } if (file.strategy === "block" && file.blockMark === undefined) { ctx.addIssue({ code: "custom", path: ["blockMark"], message: "is required when strategy is \"block\"" }); } diff --git a/test/manifest-schema.test.ts b/test/manifest-schema.test.ts index 13575c7..84e941e 100644 --- a/test/manifest-schema.test.ts +++ b/test/manifest-schema.test.ts @@ -101,6 +101,54 @@ describe("ManagedFile", () => { expect(issuePaths({ ...managedFile(), path: "package.json" }, managedFileSchema)).toContain("path"); }); + describe("reserved config paths", () => { + const reject = (path: string): string[] => issuePaths({ ...managedFile(), path }, managedFileSchema); + const accepts = (path: string): boolean => managedFileSchema.safeParse({ ...managedFile(), path }).success; + + it("rejects the root config on any extension", () => { + for (const path of ["streamctl.config.ts", "streamctl.config.mjs", "streamctl.config.js", "streamctl.config.mts"]) { + expect(reject(path), path).toContain("path"); + } + }); + + it("rejects anything under the legacy directory", () => { + expect(reject(".streamctl/config.ts")).toContain("path"); + expect(reject(".streamctl/notes.md")).toContain("path"); + }); + + // `./x`, `.//x` and `././x` all reach disk as the same file a bare `x` names, so a + // guard that only tests the raw string is bypassed by typing a prefix. + it("rejects `./`-prefixed spellings of every reservation", () => { + for (const path of [ + "./streamctl.config.ts", + ".//streamctl.config.ts", + "././streamctl.config.ts", + "./.streamctl/config.ts", + ".//.streamctl/config.ts", + "./package.json", + ".//package.json", + ]) { + expect(reject(path), path).toContain("path"); + } + }); + + // The failure mode of this guard is over-breadth, not absence: a payload's own + // wrapper files live in the same root namespace, and `acme.config.ts` is managed by + // the synthetic payload much of the suite depends on. + it("still accepts other root-level wrapper configs", () => { + for (const path of ["eslint.config.ts", "prisma.config.ts", "acme.config.ts"]) { + expect(accepts(path), path).toBe(true); + } + }); + + it("accepts the reserved names outside the invocation directory", () => { + expect(accepts("nested/streamctl.config.ts")).toBe(true); + // Same stem, different file: only `streamctl.config.` is reserved. + expect(accepts("streamctl.config-guide.md")).toBe(true); + expect(accepts("docs/streamctl.config-guide.md")).toBe(true); + }); + }); + it("rejects projectFields on a non-merge strategy", () => { expect(issuePaths({ ...managedFile(), projectFields: ["x"] }, managedFileSchema)).toContain("projectFields"); }); From b9076250dd6db0ee7f427482561720aac6e853ba Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Thu, 30 Jul 2026 16:53:04 +0200 Subject: [PATCH 12/28] feat(init): scaffold streamctl.config.ts and guard both locations init writes the config at the invocation directory and no longer creates a .streamctl/ directory. The already-initialized guard consults the resolver, so a config in either location blocks a second init and the error names the file that is actually there. --- src/engine/init.ts | 16 ++++++++++------ test/init.test.ts | 42 ++++++++++++++++++++++++++++++++---------- 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/src/engine/init.ts b/src/engine/init.ts index aeba3af..9f3a7a0 100644 --- a/src/engine/init.ts +++ b/src/engine/init.ts @@ -6,6 +6,7 @@ import type { SyncDecider, SyncPreview, SyncResult } from "./sync"; import type { LatestVersionProbe, VersionExistsProbe } from "./versions"; import { existsSync } from "node:fs"; import { join } from "node:path"; +import { resolveConfigFile } from "../config/resolve"; import { validateStreamctlConfigWithKeys } from "../config/validate"; import { StreamctlError } from "../errors"; import { stderrLogger } from "../logger"; @@ -83,7 +84,7 @@ export interface RunInitOptions { export interface InitResult { base: string; profile: Profile; - /** The payload pin written to `.streamctl/config.ts`, never the CLI's own version. */ + /** The payload pin written to `streamctl.config.ts`, never the CLI's own version. */ version: string; cliVersion: string; /** `null` when `--no-install` skipped it. */ @@ -185,7 +186,7 @@ function renderConfigTemplate(template: string, values: { package: string; base: } async function scaffoldConfig(cwd: string, template: string, values: { package: string; base: string; version: string; profile: string }): Promise { - await atomicWrite(join(cwd, ".streamctl", "config.ts"), renderConfigTemplate(template, values)); + await atomicWrite(join(cwd, "streamctl.config.ts"), renderConfigTemplate(template, values)); } /** `from === null` when the pin is newly added. */ @@ -317,7 +318,7 @@ async function resolvePayloadVersion(opts: RunInitOptions, overridden: boolean, /** * Wire a repo to a preset for the first time. Detect the profile, probe registry - * access unless skipped, scaffold `.streamctl/config.ts`, add the CLI and payload + * access unless skipped, scaffold `streamctl.config.ts`, add the CLI and payload * devDeps, install so the payload is on disk, then run the first `sync`. The CLI * hardcodes no registry. * @@ -330,10 +331,13 @@ export async function runInit(opts: RunInitOptions): Promise { if (!existsSync(join(cwd, "package.json"))) { throw new StreamctlError("NOT_A_REPO", "No package.json found. Run `streamctl init` at a repository root."); } - if (existsSync(join(cwd, ".streamctl", "config.ts"))) { + // Either location blocks a second init, and the message names the file that is + // actually there. No logger: an ambiguity warning would be noise ahead of the failure. + const existing = await resolveConfigFile(cwd); + if (existing !== null) { throw new StreamctlError( "ALREADY_INITIALIZED", - "`.streamctl/config.ts` already exists. Use `streamctl sync` or `streamctl upgrade`.", + `\`${existing.rel}\` already exists. Use \`streamctl sync\` or \`streamctl upgrade\`.`, ); } @@ -444,7 +448,7 @@ export async function runInit(opts: RunInitOptions): Promise { config, managedFiles, baseline, - // init just wrote `.streamctl/config.ts` and bumped `package.json` devDeps, so the + // init just wrote `streamctl.config.ts` and bumped `package.json` devDeps, so the // dirty-tree guard must not refuse its own first sync. allowDirty: true, decider: yes ? undefined : opts.decider, diff --git a/test/init.test.ts b/test/init.test.ts index c976093..0fa969f 100644 --- a/test/init.test.ts +++ b/test/init.test.ts @@ -1,5 +1,6 @@ import type { RunInitOptions } from "../src/engine/init"; import { execFileSync } from "node:child_process"; +import { existsSync } from "node:fs"; import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -104,12 +105,16 @@ describe("runInit", () => { expect(result.profile).toBe("nuxt-4"); expect(install).toHaveBeenCalledTimes(1); - const config = await readFile(join(repo, ".streamctl", "config.ts"), "utf8"); + const config = await readFile(join(repo, "streamctl.config.ts"), "utf8"); expect(config).toContain(`import { defineStreamctlConfig } from "@sidebase/streamctl"`); expect(config).toContain(`package: "@acme/payload"`); expect(config).toContain(`base: "nuxt-app"`); expect(config).toContain(`profile: "nuxt-4"`); expect(config).toContain(`version: "${PAYLOAD_VERSION}"`); + // `existsSync`, not this file's `readFile(...).catch(() => null)` idiom: `readFile` + // on a directory throws EISDIR, the catch swallows it, and the assertion would + // report "absent" for a `.streamctl/` sitting right there. + expect(existsSync(join(repo, ".streamctl"))).toBe(false); // `.npmrc` is a preset-managed block written by the first sync, not by init itself. const npmrc = await readFile(join(repo, ".npmrc"), "utf8"); @@ -142,7 +147,7 @@ describe("runInit", () => { await runInit(baseOpts()); - const config = await readFile(join(repo, ".streamctl", "config.ts"), "utf8"); + const config = await readFile(join(repo, "streamctl.config.ts"), "utf8"); expect(config).toContain("import { defineNuxtBaseConfig } from \"@acme/payload/config\""); expect(config).not.toContain("defineStreamctlConfig"); expect(config).toContain("package: \"@acme/payload\""); @@ -204,6 +209,23 @@ describe("runInit", () => { const error = await runInit(baseOpts()).catch((e: unknown) => e); expect(error).toBeInstanceOf(StreamctlError); expect((error as StreamctlError).code).toBe("ALREADY_INITIALIZED"); + // The legacy repo is told about the file it has, not about a root file that does + // not exist. A code-only assertion passes either way. + expect((error as StreamctlError).message).toContain(".streamctl/config.ts"); + }); + + it("rejects a root-config repo with ALREADY_INITIALIZED naming the root file", async () => { + await writeFile(join(repo, "package.json"), JSON.stringify({ name: "app", devDependencies: { nuxt: "^4.0.0" } })); + await writeFile(join(repo, "streamctl.config.ts"), "export default {}\n"); + + const error = await runInit(baseOpts()).catch((e: unknown) => e); + expect((error as StreamctlError).code).toBe("ALREADY_INITIALIZED"); + expect((error as StreamctlError).message).toContain("streamctl.config.ts"); + // Not redundant with the line above: neither path is a substring of the other, so + // the positive already distinguishes them. This catches a later copy change that + // names *both* locations — accurate for the guard, but useless to someone holding + // only one of the two files. + expect((error as StreamctlError).message).not.toContain(".streamctl/config.ts"); }); it("rejects a non-repo with NOT_A_REPO", async () => { @@ -219,7 +241,7 @@ describe("runInit", () => { expect((error as StreamctlError).code).toBe("REGISTRY_AUTH_FAILED"); expect((error as StreamctlError).message).toContain("configure registry access"); // It failed before writing the manifest, so a retry is not blocked. - expect(await readFile(join(repo, ".streamctl", "config.ts")).catch(() => null)).toBeNull(); + expect(await readFile(join(repo, "streamctl.config.ts")).catch(() => null)).toBeNull(); }); it("--skip-registry-check bypasses the probe entirely", async () => { @@ -231,7 +253,7 @@ describe("runInit", () => { expect(checkRegistryAuth).not.toHaveBeenCalled(); expect(result.base).toBe("nuxt-app"); - expect(await readFile(join(repo, ".streamctl", "config.ts"), "utf8")).toContain(`package: "@acme/payload"`); + expect(await readFile(join(repo, "streamctl.config.ts"), "utf8")).toContain(`package: "@acme/payload"`); }); it("skips the probe when pnpm.overrides already resolves the payload", async () => { @@ -280,7 +302,7 @@ describe("runInit", () => { const pkg = JSON.parse(await readFile(join(repo, "package.json"), "utf8")) as { devDependencies: Record }; expect(pkg.devDependencies["@acme/payload"]).toBe(PAYLOAD_VERSION); expect(pkg.devDependencies["@sidebase/streamctl"]).toBe(CLI_VERSION); - expect(await readFile(join(repo, ".streamctl", "config.ts"), "utf8")).toContain(`version: "${PAYLOAD_VERSION}"`); + expect(await readFile(join(repo, "streamctl.config.ts"), "utf8")).toContain(`version: "${PAYLOAD_VERSION}"`); }); it("an explicit payloadVersion wins over the probe but is still checked against the registry", async () => { @@ -317,7 +339,7 @@ describe("runInit", () => { expect((error as StreamctlError).message).toContain("--payload-version"); // The repo is left exactly as it was, so a retry isn't blocked. expect(await readFile(join(repo, "package.json"), "utf8")).toBe(before); - expect(await readFile(join(repo, ".streamctl", "config.ts")).catch(() => null)).toBeNull(); + expect(await readFile(join(repo, "streamctl.config.ts")).catch(() => null)).toBeNull(); }); it("--skip-registry-check takes an explicit payloadVersion as-is", async () => { @@ -386,7 +408,7 @@ describe("runInit", () => { expect(result.sync).toBeNull(); // Config and devDeps are still written; only the install and first sync are skipped. - expect(await readFile(join(repo, ".streamctl", "config.ts"), "utf8")).toContain(`package: "@acme/payload"`); + expect(await readFile(join(repo, "streamctl.config.ts"), "utf8")).toContain(`package: "@acme/payload"`); const pkg = JSON.parse(await readFile(join(repo, "package.json"), "utf8")) as { devDependencies: Record }; expect(pkg.devDependencies["@acme/payload"]).toBe(PAYLOAD_VERSION); expect(await readFile(join(repo, ".editorconfig")).catch(() => null)).toBeNull(); @@ -405,7 +427,7 @@ describe("runInit", () => { // is byte-identical. expect(install).not.toHaveBeenCalled(); expect(await readFile(join(repo, "package.json"), "utf8")).toBe(original); - expect(await readFile(join(repo, ".streamctl", "config.ts")).catch(() => null)).toBeNull(); + expect(await readFile(join(repo, "streamctl.config.ts")).catch(() => null)).toBeNull(); }); // nypm throws plain Errors. `upgrade` has always re-labelled them; `init` did not, so @@ -440,7 +462,7 @@ describe("runInit", () => { const result = await runInit(baseOpts({ install: createInstaller({ confirm: async () => false }) })); expect(result.sync).toBeNull(); - expect(await readFile(join(repo, ".streamctl", "config.ts"), "utf8")).toContain(`package: "@acme/payload"`); + expect(await readFile(join(repo, "streamctl.config.ts"), "utf8")).toContain(`package: "@acme/payload"`); expect(await readFile(join(repo, ".editorconfig")).catch(() => null)).toBeNull(); }); }); @@ -476,7 +498,7 @@ describe("runInit (v2 manifest auto-detection)", () => { // Profile from `profiles[].detect`, base from `defaultBase`. expect(result.profile).toBe("nuxt-4"); expect(result.base).toBe("nuxt-app"); - expect(await readFile(join(repo, ".streamctl", "config.ts"), "utf8")).toContain(`profile: "nuxt-4"`); + expect(await readFile(join(repo, "streamctl.config.ts"), "utf8")).toContain(`profile: "nuxt-4"`); // Detection is never silent: the evidence is logged. expect(warn).toHaveBeenCalledWith(expect.stringContaining("detected profile \"nuxt-4\"")); expect(warn).toHaveBeenCalledWith(expect.stringContaining("nuxt ^4.2.0 in devDependencies")); From 38c449f8bcaf1ea22435df29bcc97e56ca2af17a Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Thu, 30 Jul 2026 16:54:47 +0200 Subject: [PATCH 13/28] test: guard the manifest reservations against resolver drift Also pins details.path on the ambiguous-pin branch and corrects the posix.normalize rationale: on Windows the platform normalize would turn the legacy prefix into a backslash path, silently disabling that reservation rather than tripping the backslash check. --- src/manifest/schema.ts | 13 +++++++++---- test/manifest-schema.test.ts | 11 +++++++++++ test/upgrade.test.ts | 6 ++++++ 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/manifest/schema.ts b/src/manifest/schema.ts index 0512457..4e3713f 100644 --- a/src/manifest/schema.ts +++ b/src/manifest/schema.ts @@ -83,10 +83,15 @@ export const managedFileSchema = z.strictObject({ // `././x` all reach disk as `join(cwd, path)` — the same file a bare `x` names, so an // equality or prefix test on the raw value is trivially bypassed. // - // `posix.normalize`, never the platform `normalize`: the latter emits backslashes on - // Windows, which the check three lines above rejects. It also leaves a leading `..` - // in place, so the `..` check keeps firing on the raw path; a doubly-bad path simply - // collects two issues, which is why this is not an early return. + // `posix.normalize`, never the platform `normalize`. On Windows the latter turns + // `.streamctl/config.ts` into `.streamctl\config.ts`, so the legacy prefix test below + // silently stops matching and the reservation disappears with no issue raised — the + // backslash check above does not save it, because that runs on the raw `file.path`, + // which has no backslash. Verified against `win32.normalize`. + // + // Normalization also leaves a leading `..` in place, so the `..` check keeps firing on + // the raw path; a doubly-bad path simply collects two issues, which is why this is not + // an early return. const reserved = posix.normalize(file.path); // `package.json` is owned by the version reconcile (writes it last from a pre-write // snapshot); managing it as a file too would let the reconcile silently clobber that write. diff --git a/test/manifest-schema.test.ts b/test/manifest-schema.test.ts index 84e941e..d85e1f5 100644 --- a/test/manifest-schema.test.ts +++ b/test/manifest-schema.test.ts @@ -1,5 +1,6 @@ import type { ZodError } from "zod"; import { describe, expect, it } from "vitest"; +import { CONFIG_FILE, LEGACY_CONFIG_FILE } from "../src/config/resolve"; import { managedFileSchema, payloadManifestSchema, @@ -141,6 +142,16 @@ describe("ManagedFile", () => { } }); + // The schema duplicates these spellings rather than importing them, because the + // published `./manifest` entry point must not acquire a path into the loader (which + // imports c12). A test has no such constraint, so it can hold the two in agreement: + // renaming a constant in `resolve.ts` would otherwise silently un-reserve the path + // while this file's own cases kept passing. + it("stays in agreement with the resolver's spellings", () => { + expect(reject(`${CONFIG_FILE}.ts`)).toContain("path"); + expect(reject(`${LEGACY_CONFIG_FILE}.ts`)).toContain("path"); + }); + it("accepts the reserved names outside the invocation directory", () => { expect(accepts("nested/streamctl.config.ts")).toBe(true); // Same stem, different file: only `streamctl.config.` is reserved. diff --git a/test/upgrade.test.ts b/test/upgrade.test.ts index 6369579..67746b2 100644 --- a/test/upgrade.test.ts +++ b/test/upgrade.test.ts @@ -713,6 +713,12 @@ describe("runUpgrade", () => { expect(error).toBeInstanceOf(StreamctlError); expect((error as StreamctlError).code).toBe("CONFIG_INVALID"); expect((error as StreamctlError).message).toMatch(/ambiguous version pin/i); + // This repo uses the root config, so it catches a hardcoded *legacy* literal in the + // ambiguous branch — the mirror of the legacy-repo test on the no-pin branch, which + // catches a hardcoded root one. Between them both branches and both directions are + // covered; the unreadable-file branch is deliberately left unpinned, since only a + // TOCTOU delete between the resolver's probe and the read can reach it. + expect(((error as StreamctlError).details as { path: string }).path).toBe("streamctl.config.ts"); // Guessing between the two would corrupt one of them, so we write nothing. expect(await readConfig()).toBe(ambiguous); From 54503e5615746877a5a069faba2fd55b5411da1f Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Thu, 30 Jul 2026 17:13:49 +0200 Subject: [PATCH 14/28] fix(manifest): match reserved config paths case-insensitively A case variant such as PACKAGE.JSON or .STREAMCTL/config.ts named the reserved file on macOS and Windows while passing the guard, letting a payload manage the config that sync then overwrites. Lowercasing inside the shared normalization covers all three reservations at once. Also label the reservation assertions with the path under test: the failure fires inside the issuePaths helper, so a call-site label never renders. --- src/manifest/schema.ts | 15 ++++++++++++- test/manifest-schema.test.ts | 42 ++++++++++++++++++++++++++++++------ 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/src/manifest/schema.ts b/src/manifest/schema.ts index 4e3713f..073b5e9 100644 --- a/src/manifest/schema.ts +++ b/src/manifest/schema.ts @@ -92,7 +92,20 @@ export const managedFileSchema = z.strictObject({ // Normalization also leaves a leading `..` in place, so the `..` check keeps firing on // the raw path; a doubly-bad path simply collects two issues, which is why this is not // an early return. - const reserved = posix.normalize(file.path); + // + // Lowercased deliberately, and shared by all three reservations below. macOS defaults to + // a case-insensitive filesystem and Windows is always case-insensitive, so `PACKAGE.JSON` + // and `.STREAMCTL/config.ts` name the reserved files there — managing one would have + // `sync` overwrite the file the same run wrote. All comparands are lowercase. + // + // This over-rejects on Linux, where `STREAMCTL.CONFIG.TS` really is a different file: + // accepted, because a manifest is portable and must validate identically everywhere. A + // platform-conditional check would make a payload valid on CI and invalid on a laptop. + // + // `toLowerCase`, never `toLocaleLowerCase`: the locale-aware form maps `I` to dotless + // `ı` under a Turkish locale, which would un-reserve `STREAMCTL.CONFIG.TS` for exactly + // those users. + const reserved = posix.normalize(file.path).toLowerCase(); // `package.json` is owned by the version reconcile (writes it last from a pre-write // snapshot); managing it as a file too would let the reconcile silently clobber that write. if (reserved === "package.json") { diff --git a/test/manifest-schema.test.ts b/test/manifest-schema.test.ts index d85e1f5..5b7dd35 100644 --- a/test/manifest-schema.test.ts +++ b/test/manifest-schema.test.ts @@ -22,10 +22,15 @@ function payload(): Record { return { schemaVersion: 2, presets: ["base", "nuxt-app"], profiles: [{ name: "nuxt-4" }], defaultBase: "nuxt-app" }; } -/** Every issue `path` from a parse that is expected to fail. */ -function issuePaths(input: unknown, schema: { safeParse: (v: unknown) => { success: boolean; error?: ZodError } }): string[] { +/** + * Every issue `path` from a parse that is expected to fail. `label` is passed to the + * inner assertion, not just the caller's: a reservation that stops matching makes the + * parse *succeed*, so this line is what fails, and a label on the caller's `toContain` + * never gets a chance to print. + */ +function issuePaths(input: unknown, schema: { safeParse: (v: unknown) => { success: boolean; error?: ZodError } }, label?: string): string[] { const result = schema.safeParse(input); - expect(result.success).toBe(false); + expect(result.success, label).toBe(false); return zodToIssues(result.error as ZodError).map(issue => issue.path); } @@ -103,7 +108,7 @@ describe("ManagedFile", () => { }); describe("reserved config paths", () => { - const reject = (path: string): string[] => issuePaths({ ...managedFile(), path }, managedFileSchema); + const reject = (path: string): string[] => issuePaths({ ...managedFile(), path }, managedFileSchema, path); const accepts = (path: string): boolean => managedFileSchema.safeParse({ ...managedFile(), path }).success; it("rejects the root config on any extension", () => { @@ -133,11 +138,29 @@ describe("ManagedFile", () => { } }); + // Case-insensitive on purpose: on macOS (by default) and Windows (always) these name + // the reserved files, so a payload could manage the config through a case variant and + // have `sync` overwrite it. `package.json` is covered by the same normalization. + it("rejects case variants of every reservation", () => { + for (const path of [ + "STREAMCTL.CONFIG.TS", + "Streamctl.Config.ts", + ".STREAMCTL/config.ts", + ".Streamctl/notes.md", + "PACKAGE.JSON", + "./STREAMCTL.CONFIG.TS", + ]) { + expect(reject(path), path).toContain("path"); + } + }); + // The failure mode of this guard is over-breadth, not absence: a payload's own // wrapper files live in the same root namespace, and `acme.config.ts` is managed by // the synthetic payload much of the suite depends on. + // Lowercasing the compared path widens the match by construction, so these are the + // cases that bound it. it("still accepts other root-level wrapper configs", () => { - for (const path of ["eslint.config.ts", "prisma.config.ts", "acme.config.ts"]) { + for (const path of ["eslint.config.ts", "prisma.config.ts", "acme.config.ts", "ESLint.config.ts"]) { expect(accepts(path), path).toBe(true); } }); @@ -148,8 +171,13 @@ describe("ManagedFile", () => { // renaming a constant in `resolve.ts` would otherwise silently un-reserve the path // while this file's own cases kept passing. it("stays in agreement with the resolver's spellings", () => { - expect(reject(`${CONFIG_FILE}.ts`)).toContain("path"); - expect(reject(`${LEGACY_CONFIG_FILE}.ts`)).toContain("path"); + // `reject` labels its assertions with the path. That matters most here: a rename + // reds ~66 tests across six files, all of them "resolution moved" and all green + // again once fixtures follow the new spelling. This one stays red until + // `CONFIG_STEM` moves too, so it is the last failure standing and the easiest to + // mis-read as collateral. The label names the spelling that drifted. + expect(reject(`${CONFIG_FILE}.ts`), `${CONFIG_FILE}.ts`).toContain("path"); + expect(reject(`${LEGACY_CONFIG_FILE}.ts`), `${LEGACY_CONFIG_FILE}.ts`).toContain("path"); }); it("accepts the reserved names outside the invocation directory", () => { From 4121810062f3cc8102f07ffb997377a79974a111 Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Thu, 30 Jul 2026 17:13:49 +0200 Subject: [PATCH 15/28] test: pin init's guard ordering and silent both-present failure --- test/init.test.ts | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/test/init.test.ts b/test/init.test.ts index 0fa969f..64f6e2a 100644 --- a/test/init.test.ts +++ b/test/init.test.ts @@ -8,6 +8,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { bumpDevDeps, readPayloadOverride, runInit } from "../src/engine/init"; import { createInstaller } from "../src/engine/pm"; import { StreamctlError } from "../src/errors"; +import { captureStderr } from "./helpers/streams"; // Deliberately different numbers. The CLI and the payload release independently, so a // test that passes with one shared constant would hide a re-coupling of the two pins. @@ -234,6 +235,43 @@ describe("runInit", () => { expect((error as StreamctlError).code).toBe("NOT_A_REPO"); }); + it("checks for a repo before checking for a config", async () => { + // A config with no `package.json` must still fail NOT_A_REPO. The only other + // NOT_A_REPO test has neither file, so it cannot see the ordering — and the config + // guard is now an awaited resolver call doing up to 24 stats, which invites being + // hoisted above the cheap synchronous probe. + await writeFile(join(repo, "streamctl.config.ts"), "export default {}\n"); + + const error = await runInit(baseOpts()).catch((e: unknown) => e); + expect((error as StreamctlError).code).toBe("NOT_A_REPO"); + }); + + it("blocks a both-present repo without emitting the ambiguity warning", async () => { + // The one deliberate deviation in the feature: `resolveConfigFile` takes an optional + // logger with no `stderrLogger` fallback, and `init` passes none, so a warning cannot + // precede the failure. That has two independent halves, and they need two channels: + // the spy proves `init` does not forward its own logger; the stderr capture proves + // the resolver has no house default behind it. A spy alone is blind to + // `(logger ?? stderrLogger).warn(...)`, which writes past it to the real stream. + await writeFile(join(repo, "package.json"), JSON.stringify({ name: "app", devDependencies: { nuxt: "^4.0.0" } })); + await writeFile(join(repo, "streamctl.config.ts"), "export default {}\n"); + await mkdir(join(repo, ".streamctl"), { recursive: true }); + await writeFile(join(repo, ".streamctl", "config.ts"), "export default {}\n"); + const warn = vi.fn(); + const stderr = captureStderr(); + + const error = await runInit(baseOpts({ logger: { warn } })).catch((e: unknown) => e); + + // Root wins, so the message names the file `init` would otherwise have written. + expect((error as StreamctlError).code).toBe("ALREADY_INITIALIZED"); + expect((error as StreamctlError).message).toContain("streamctl.config.ts"); + expect(stderr.join("")).toBe(""); + expect(warn).not.toHaveBeenCalledWith(expect.stringContaining("streamctl:")); + // Also catches a reworded warning that drops the prefix. `init`'s legitimate warnings + // (profile detection) never name a config path, so this cannot false-fire. + expect(warn).not.toHaveBeenCalledWith(expect.stringMatching(/streamctl\.config\.ts|\.streamctl\/config\.ts/u)); + }); + it("surfaces REGISTRY_AUTH_FAILED when packages are unreadable", async () => { await writeFile(join(repo, "package.json"), JSON.stringify({ name: "app", devDependencies: { nuxt: "^4.0.0" } })); const error = await runInit(baseOpts({ checkRegistryAuth: async () => false })).catch((e: unknown) => e); From 4aabe0a46ec295afd7f16d06e5af39906870b24e Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Thu, 30 Jul 2026 17:30:43 +0200 Subject: [PATCH 16/28] refactor(init): bind the scaffold path to CONFIG_FILE A literal spelling in the write path let init scaffold a file the resolver no longer looks for. Also assert stderr in the resolver's no-logger test, which watched only the injected array and stayed green under the house-default fallback it exists to forbid. --- src/engine/init.ts | 7 +++++-- test/resolve.test.ts | 14 +++++++++++++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/engine/init.ts b/src/engine/init.ts index 9f3a7a0..74465b4 100644 --- a/src/engine/init.ts +++ b/src/engine/init.ts @@ -6,7 +6,7 @@ import type { SyncDecider, SyncPreview, SyncResult } from "./sync"; import type { LatestVersionProbe, VersionExistsProbe } from "./versions"; import { existsSync } from "node:fs"; import { join } from "node:path"; -import { resolveConfigFile } from "../config/resolve"; +import { CONFIG_FILE, resolveConfigFile } from "../config/resolve"; import { validateStreamctlConfigWithKeys } from "../config/validate"; import { StreamctlError } from "../errors"; import { stderrLogger } from "../logger"; @@ -186,7 +186,10 @@ function renderConfigTemplate(template: string, values: { package: string; base: } async function scaffoldConfig(cwd: string, template: string, values: { package: string; base: string; version: string; profile: string }): Promise { - await atomicWrite(join(cwd, "streamctl.config.ts"), renderConfigTemplate(template, values)); + // Bound to the constant, not spelled out: `resolve.ts` is the single authority on + // where a config lives, and a literal here would let `init` scaffold a file the + // resolver no longer looks for. + await atomicWrite(join(cwd, `${CONFIG_FILE}.ts`), renderConfigTemplate(template, values)); } /** `from === null` when the pin is newly added. */ diff --git a/test/resolve.test.ts b/test/resolve.test.ts index 7d5d97e..c63a855 100644 --- a/test/resolve.test.ts +++ b/test/resolve.test.ts @@ -4,8 +4,9 @@ import { tmpdir } from "node:os"; import { dirname, isAbsolute, join, resolve } from "node:path"; import { pathToFileURL } from "node:url"; import { SUPPORTED_EXTENSIONS } from "c12"; -import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { CONFIG_FILE, configCandidates, LEGACY_CONFIG_FILE, resolveConfigFile } from "../src/config/resolve"; +import { captureStderr } from "./helpers/streams"; /** * Does `chmod 0o000` on a directory actually revoke traversal here? Windows can't @@ -43,6 +44,10 @@ beforeEach(async () => { }); afterEach(async () => { + // This file has no other spies, but `captureStderr` installs one and vitest is not + // configured with `restoreMocks`, so without this the stub leaks into every later + // test in the file and swallows output silently. + vi.restoreAllMocks(); await rm(root, { recursive: true, force: true }); }); @@ -134,10 +139,17 @@ describe("resolveConfigFile", () => { }); it("emits nothing when no logger is passed", async () => { + // Both channels, because each is blind to the other's failure. `warnings` is the + // injected array, which stays empty under `(logger ?? stderrLogger).warn(...)` — + // that writes past it to the real stream. The capture is what sees the house + // default, and the house default is the deviation this test exists to guard. + const stderr = captureStderr(); + const location = await resolveConfigFile(root); expect(location?.source).toBe("root"); expect(warnings).toEqual([]); + expect(stderr.join("")).toBe(""); }); it("evaluates neither config module", async () => { From 3d09e4235125552e089be19bd73de98560287ab4 Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Thu, 30 Jul 2026 17:34:51 +0200 Subject: [PATCH 17/28] docs(test): record what captureStderr can and cannot see --- test/helpers/streams.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/helpers/streams.ts b/test/helpers/streams.ts index b110839..ac5b6f5 100644 --- a/test/helpers/streams.ts +++ b/test/helpers/streams.ts @@ -6,6 +6,18 @@ import { vi } from "vitest"; * read the writes back. The later `vi.spyOn` replaces the earlier stub, and * `vi.restoreAllMocks()` in `afterEach` undoes both. * + * **Sees `process.stdout`/`process.stderr.write` and nothing else.** `console.*` is + * intercepted by vitest before it reaches the stream, a child process's inherited stdio + * bypasses it, and so does a raw `fs.writeSync(2, …)`. So an "asserts nothing was + * written" test is only as strong as the rule that `src/` never calls `console.*` + * directly — true today (`rg 'console\.(error|warn|log)' src/` returns nothing) and the + * reason every diagnostic goes through `Logger`. If that rule ever slips, these + * assertions stop guarding without failing. + * + * A caller also needs `vi.restoreAllMocks()` in its own `afterEach`: vitest is not + * configured with `restoreMocks`, so the stub otherwise leaks into every later test in + * the file and swallows output silently. + * * `test/init.command.test.ts:12-20` duplicates `captureStdout` rather than importing it, * deliberately: that file is a regression witness for this feature and has to stay * untouched across the whole branch, so even adding this note to it would break the From a5926eaec507d135278b81777ced8c5c55da9985 Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Thu, 30 Jul 2026 17:42:07 +0200 Subject: [PATCH 18/28] test: move fixtures to the root config location Mechanical, safe to scan: four config fixtures relocated with git mv (renames, zero-line diffs), the two command temp-repo builders repointed, and load.test.ts's location tests split into a root case and a legacy one against the new fixture. Four additions worth reading closely: - a loader-level eval-once test. resolve.test.ts asserts the same property, but the resolver only stats and cannot violate it; loadConfig can. Under a two-call loader this is the only failure in 711 tests. - legacy-vs-root parity: the same command against one repo converted in place, asserting identical report output and empty stderr. - sideEffectConfig promoted to test/helpers, with a VALID_BODY constant because its default export fails validation one layer up. - a tripwire keeping upgrade's default fixture at the root, which is the only thing that can detect a legacy-hardcoded pin bump. --- .../config.ts => streamctl.config.ts} | 0 .../config.ts => streamctl.config.ts} | 0 .../both-present/.streamctl/config.ts | 6 ++ .../fixtures/both-present/streamctl.config.ts | 6 ++ .../legacy-valid/.streamctl/config.ts | 6 ++ .../config.ts => streamctl.config.ts} | 0 .../config.ts => streamctl.config.ts} | 0 test/helpers/configs.ts | 19 ++++++ test/load.test.ts | 64 ++++++++++++++++++- test/resolve.test.ts | 6 +- test/status.command.test.ts | 34 +++++++++- test/sync-check.command.test.ts | 11 ++-- test/upgrade.test.ts | 28 +++++++- 13 files changed, 161 insertions(+), 19 deletions(-) rename test/fixtures/bad-exclude/{.streamctl/config.ts => streamctl.config.ts} (100%) rename test/fixtures/bad-semver/{.streamctl/config.ts => streamctl.config.ts} (100%) create mode 100644 test/fixtures/both-present/.streamctl/config.ts create mode 100644 test/fixtures/both-present/streamctl.config.ts create mode 100644 test/fixtures/legacy-valid/.streamctl/config.ts rename test/fixtures/unknown-key/{.streamctl/config.ts => streamctl.config.ts} (100%) rename test/fixtures/valid/{.streamctl/config.ts => streamctl.config.ts} (100%) create mode 100644 test/helpers/configs.ts diff --git a/test/fixtures/bad-exclude/.streamctl/config.ts b/test/fixtures/bad-exclude/streamctl.config.ts similarity index 100% rename from test/fixtures/bad-exclude/.streamctl/config.ts rename to test/fixtures/bad-exclude/streamctl.config.ts diff --git a/test/fixtures/bad-semver/.streamctl/config.ts b/test/fixtures/bad-semver/streamctl.config.ts similarity index 100% rename from test/fixtures/bad-semver/.streamctl/config.ts rename to test/fixtures/bad-semver/streamctl.config.ts diff --git a/test/fixtures/both-present/.streamctl/config.ts b/test/fixtures/both-present/.streamctl/config.ts new file mode 100644 index 0000000..11e31a4 --- /dev/null +++ b/test/fixtures/both-present/.streamctl/config.ts @@ -0,0 +1,6 @@ +export default { + package: "@acme/payload", + base: "legacy-base", + version: "1.1.1", + profile: "nuxt-4", +}; diff --git a/test/fixtures/both-present/streamctl.config.ts b/test/fixtures/both-present/streamctl.config.ts new file mode 100644 index 0000000..d78f733 --- /dev/null +++ b/test/fixtures/both-present/streamctl.config.ts @@ -0,0 +1,6 @@ +export default { + package: "@acme/payload", + base: "nuxt-app", + version: "9.9.9", + profile: "nuxt-4", +}; diff --git a/test/fixtures/legacy-valid/.streamctl/config.ts b/test/fixtures/legacy-valid/.streamctl/config.ts new file mode 100644 index 0000000..964f4c7 --- /dev/null +++ b/test/fixtures/legacy-valid/.streamctl/config.ts @@ -0,0 +1,6 @@ +export default { + package: "@acme/payload", + base: "nuxt-app", + version: "1.2.3", + profile: "nuxt-4", +}; diff --git a/test/fixtures/unknown-key/.streamctl/config.ts b/test/fixtures/unknown-key/streamctl.config.ts similarity index 100% rename from test/fixtures/unknown-key/.streamctl/config.ts rename to test/fixtures/unknown-key/streamctl.config.ts diff --git a/test/fixtures/valid/.streamctl/config.ts b/test/fixtures/valid/streamctl.config.ts similarity index 100% rename from test/fixtures/valid/.streamctl/config.ts rename to test/fixtures/valid/streamctl.config.ts diff --git a/test/helpers/configs.ts b/test/helpers/configs.ts new file mode 100644 index 0000000..c7ca079 --- /dev/null +++ b/test/helpers/configs.ts @@ -0,0 +1,19 @@ +/** + * A config whose module body touches the filesystem when evaluated. Writing the sentinel + * is the only observable difference between "this file was read" and "this file was + * loaded", which is what both the resolver and the loader need to prove. + * + * A test using this must also prove the fixture is not inert — a body that silently + * fails to write would make every "did not evaluate" assertion pass for the wrong + * reason. See `test/resolve.test.ts`'s proof phase for the pattern. + */ +export function sideEffectConfig(sentinel: string, defaultExport = "{}"): string { + return `import { writeFileSync } from "node:fs";\nwriteFileSync(${JSON.stringify(sentinel)}, "");\nexport default ${defaultExport}\n`; +} + +/** + * A valid config body, for callers that reach validation. `sideEffectConfig`'s default + * `{}` fails it — fine for the resolver, which never validates, but a loader-level test + * needs a config that survives the whole pipeline or it fails before proving anything. + */ +export const VALID_BODY = `{ package: "@acme/payload", base: "nuxt-app", version: "1.2.3", profile: "nuxt-4" }`; diff --git a/test/load.test.ts b/test/load.test.ts index 7752639..720d520 100644 --- a/test/load.test.ts +++ b/test/load.test.ts @@ -1,4 +1,4 @@ -import { mkdtempSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { existsSync, mkdtempSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; import { mkdir, mkdtemp, rm, symlink, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; @@ -7,6 +7,7 @@ import { loadConfig } from "c12"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { loadStreamctlConfig } from "../src/config/load"; import { StreamctlError } from "../src/errors"; +import { sideEffectConfig, VALID_BODY } from "./helpers/configs"; const fixtures = join(dirname(fileURLToPath(import.meta.url)), "fixtures"); @@ -43,6 +44,31 @@ describe("loadStreamctlConfig", () => { expect(config.versionSyncExclude).toEqual(["devDependencies.typescript"]); }); + // The temp-dir tests below already assert `source === "legacy"`. What these two add is + // a committed fixture on disk: the layout an adopter actually has, exercised through + // the real loader rather than through a directory the test just built. + it("loads a committed legacy fixture", async () => { + const { config, location } = await loadStreamctlConfig(join(fixtures, "legacy-valid")); + expect(config.base).toBe("nuxt-app"); + expect(location.source).toBe("legacy"); + expect(location.rel).toBe(".streamctl/config.ts"); + }); + + it("prefers the root config in a committed both-present fixture", async () => { + const warnings: string[] = []; + + const { config, location } = await loadStreamctlConfig(join(fixtures, "both-present"), { + logger: { warn: message => warnings.push(message) }, + }); + + // The two files differ in `base` and `version`, so these are proof of *which* was + // read, not merely that something loaded. + expect(config.base).toBe("nuxt-app"); + expect(config.version).toBe("9.9.9"); + expect(location.source).toBe("root"); + expect(warnings).toHaveLength(1); + }); + it("NOT_INITIALIZED when the config is absent", async () => { const error = await loadStreamctlConfig(join(fixtures, "uninitialized")).catch((e: unknown) => e); expect(error).toBeInstanceOf(StreamctlError); @@ -74,10 +100,19 @@ describe("loadStreamctlConfig", () => { }); describe("location", () => { - it("reports the legacy location for a legacy fixture", async () => { + it("reports the root location for a root fixture", async () => { const cwd = join(fixtures, "valid"); const { location } = await loadStreamctlConfig(cwd); + expect(location.source).toBe("root"); + expect(location.rel).toBe("streamctl.config.ts"); + expect(location.abs).toBe(join(cwd, "streamctl.config.ts")); + }); + + it("reports the legacy location for a legacy fixture", async () => { + const cwd = join(fixtures, "legacy-valid"); + const { location } = await loadStreamctlConfig(cwd); + expect(location.source).toBe("legacy"); expect(location.rel).toBe(".streamctl/config.ts"); expect(location.abs).toBe(join(cwd, ".streamctl", "config.ts")); @@ -90,7 +125,10 @@ describe("loadStreamctlConfig", () => { const { _configFile } = await loadConfig({ cwd, name: "streamctl", - configFile: ".streamctl/config", + // The spelling the loader used for this fixture. Must track the fixture's + // location: point it at the other one and c12 resolves nothing, `_configFile` + // is undefined, and the comparison below degrades into realpath("") throwing. + configFile: "streamctl.config", rcFile: false, globalRc: false, packageJson: false, @@ -145,6 +183,26 @@ describe("loadStreamctlConfig", () => { expect(warnings[0]).toContain(".streamctl/config.ts"); }); + it("evaluates the root config and not the legacy one", async () => { + // The layer that can actually violate this. `test/resolve.test.ts` asserts the same + // property, but the resolver only stats — it has no way to evaluate anything, so + // that test guards where the risk isn't. `loadConfig` is the call that evaluates, + // and a two-call loader would evaluate both modules while still returning the root + // result: every assertion in the test above stays true. Measured at P01-V01. + const rootSentinel = join(root, "root-evaluated"); + const legacySentinel = join(root, "legacy-evaluated"); + await writeFile(join(root, "streamctl.config.ts"), sideEffectConfig(rootSentinel, VALID_BODY)); + await mkdir(join(root, ".streamctl"), { recursive: true }); + await writeFile(join(root, ".streamctl", "config.ts"), sideEffectConfig(legacySentinel, VALID_BODY)); + + await loadStreamctlConfig(root, { logger: { warn: () => {} } }); + + // Positive first: it proves the fixture works, so the negative below cannot pass + // because the body silently failed to write. + expect(existsSync(rootSentinel)).toBe(true); + expect(existsSync(legacySentinel)).toBe(false); + }); + it.skipIf(!canSymlink)("accepts a config that is a symlink", async () => { // `statSync` follows the link, so the probe reports the link path while c12 may // realpath to the target. The loader's invariant compares realpath'd forms, so diff --git a/test/resolve.test.ts b/test/resolve.test.ts index c63a855..753783b 100644 --- a/test/resolve.test.ts +++ b/test/resolve.test.ts @@ -6,6 +6,7 @@ import { pathToFileURL } from "node:url"; import { SUPPORTED_EXTENSIONS } from "c12"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { CONFIG_FILE, configCandidates, LEGACY_CONFIG_FILE, resolveConfigFile } from "../src/config/resolve"; +import { sideEffectConfig } from "./helpers/configs"; import { captureStderr } from "./helpers/streams"; /** @@ -67,11 +68,6 @@ async function writeConfigDirFile(rel: string, body = "export default {}\n"): Pr await writeFile(abs, body); } -/** A config whose module body touches the filesystem when evaluated. */ -function sideEffectConfig(sentinel: string): string { - return `import { writeFileSync } from "node:fs";\nwriteFileSync(${JSON.stringify(sentinel)}, "");\nexport default {}\n`; -} - describe("resolveConfigFile", () => { it("resolves a root config", async () => { await writeRootConfig(); diff --git a/test/status.command.test.ts b/test/status.command.test.ts index cd61e51..c1caa58 100644 --- a/test/status.command.test.ts +++ b/test/status.command.test.ts @@ -3,6 +3,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { statusCommand } from "../src/commands/status"; +import { captureStderr } from "./helpers/streams"; type StatusArgs = Parameters>[0]; @@ -21,6 +22,8 @@ const TEMPLATES: Record = { "base/npmrc": "registry=https://example\n", }; +const CONFIG_BODY = `export default { package: "@acme/payload", base: "base", version: "${VERSION}", profile: "nuxt-4" };\n`; + let repo: string; const previousExitCode = process.exitCode; const stdout: string[] = []; @@ -32,8 +35,7 @@ async function makeRepo(): Promise { for (const [source, content] of Object.entries(TEMPLATES)) { await writeFile(join(pkg, "presets", source), content); } - await mkdir(join(repo, ".streamctl"), { recursive: true }); - await writeFile(join(repo, ".streamctl", "config.ts"), `export default { package: "@acme/payload", base: "base", version: "${VERSION}", profile: "nuxt-4" };\n`); + await writeFile(join(repo, "streamctl.config.ts"), CONFIG_BODY); } beforeEach(async () => { @@ -104,9 +106,35 @@ describe("status command", () => { }); it("exits 1 when the repo has no config", async () => { - await rm(join(repo, ".streamctl"), { recursive: true, force: true }); + await rm(join(repo, "streamctl.config.ts"), { force: true }); await statusCommand.run?.({ args: {} } as unknown as StatusArgs); expect(process.exitCode).toBe(1); }); + + // The automated guard on the feature's headline promise: a legacy repo behaves exactly + // as it does today. Everything else in the suite checks the new location works; this is + // the one that checks the old one did not quietly change. Same repo converted in place + // rather than two temp dirs, so the two runs differ in the config's location and in + // nothing else — not even the tmpdir name, which would otherwise show up in the diff. + it("reports identically from either config location", async () => { + await statusCommand.run?.({ args: { json: true } } as unknown as StatusArgs); + const fromRoot = JSON.parse(stdout.join("")) as unknown; + + await rm(join(repo, "streamctl.config.ts")); + await mkdir(join(repo, ".streamctl"), { recursive: true }); + await writeFile(join(repo, ".streamctl", "config.ts"), CONFIG_BODY); + stdout.length = 0; + // Replaces the `beforeEach` stub; `vi.restoreAllMocks()` in `afterEach` undoes both. + const stderr = captureStderr(); + + await statusCommand.run?.({ args: { json: true } } as unknown as StatusArgs); + const fromLegacy = JSON.parse(stdout.join("")) as unknown; + + expect(fromLegacy).toEqual(fromRoot); + expect(process.exitCode).toBe(0); + // No deprecation notice, no ambiguity warning: the legacy path is supported outright, + // so a legacy user sees nothing a root user wouldn't. + expect(stderr.join("")).toBe(""); + }); }); diff --git a/test/sync-check.command.test.ts b/test/sync-check.command.test.ts index 2c6cf5a..62ea2fd 100644 --- a/test/sync-check.command.test.ts +++ b/test/sync-check.command.test.ts @@ -33,7 +33,7 @@ const TEMPLATES: Record = { let repo: string; const previousExitCode = process.exitCode; -/** Build a temp consuming repo: installed config payload + a valid .streamctl/config.ts. */ +/** Build a temp consuming repo: installed config payload + a valid streamctl.config.ts. */ async function makeRepo(): Promise { // Keep the lockfile walk inside the fixture (see test/pm.test.ts); a stray // lockfile above the tmpdir would print a false stale-lockfile hint. @@ -44,8 +44,7 @@ async function makeRepo(): Promise { for (const [source, content] of Object.entries(TEMPLATES)) { await writeFile(join(pkg, "presets", source), content); } - await mkdir(join(repo, ".streamctl"), { recursive: true }); - await writeFile(join(repo, ".streamctl", "config.ts"), `export default { package: "@acme/payload", base: "base", version: "${VERSION}", profile: "nuxt-4" };\n`); + await writeFile(join(repo, "streamctl.config.ts"), `export default { package: "@acme/payload", base: "base", version: "${VERSION}", profile: "nuxt-4" };\n`); } /** The consumer's `.vscode/settings.json`: `editor` is a string, the payload declares an object. */ @@ -217,14 +216,14 @@ describe("sync + check commands (exit codes)", () => { expect(out.join("")).not.toContain(tmpdir()); }); - // A typo'd profile in .streamctl/config.ts used to resolve the version baseline to + // A typo'd profile in streamctl.config.ts used to resolve the version baseline to // `{}`, silently disabling reconcile forever behind a soft detect warning. it.each([ { command: "sync", run: async () => syncCommand.run?.({ args: {} } as unknown as SyncArgs) }, { command: "check", run: async () => checkCommand.run?.({ args: { "fail-on": "drift" } } as unknown as CheckArgs) }, ])("$command exits 1 on a profile the payload does not declare", async ({ run }) => { await writeFile( - join(repo, ".streamctl", "config.ts"), + join(repo, "streamctl.config.ts"), `export default { package: "@acme/payload", base: "base", version: "${VERSION}", profile: "n5" };\n`, ); @@ -234,7 +233,7 @@ describe("sync + check commands (exit codes)", () => { it("sync --json names the undeclared profile and the declared ones", async () => { await writeFile( - join(repo, ".streamctl", "config.ts"), + join(repo, "streamctl.config.ts"), `export default { package: "@acme/payload", base: "base", version: "${VERSION}", profile: "n5" };\n`, ); const out: string[] = []; diff --git a/test/upgrade.test.ts b/test/upgrade.test.ts index 67746b2..ef482ba 100644 --- a/test/upgrade.test.ts +++ b/test/upgrade.test.ts @@ -8,6 +8,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { promisify } from "node:util"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { CONFIG_FILE } from "../src/config/resolve"; import { extractEmbeddedVersion } from "../src/engine/init"; import { runUpgrade } from "../src/engine/upgrade"; import { StreamctlError } from "../src/errors"; @@ -49,7 +50,17 @@ async function makeConfigPackage(version: string): Promise { } } -const writeConfig = (content: string): Promise => writeFile(join(repo, "streamctl.config.ts"), content); +/** + * The default fixture is at the ROOT, and that is load-bearing — do not relocate it. + * + * The `runUpgrade: legacy config location` block below cannot detect a `bumpConfigVersion` + * hardcoded back to the legacy path: it passes 4/4 under a full revert, including the + * byte-for-byte rollback case, because from inside a legacy repo the hash oracle cannot + * tell "rollback restored it" from "nothing ever wrote it". Detection comes entirely from + * the root-repo tests in this file. Move this to legacy and they stop being able to see + * it, while everything here stays green. + */ +const writeConfig = (content: string): Promise => writeFile(join(repo, `${CONFIG_FILE}.ts`), content); /** A clean initialized repo pinned to FROM (config.ts + devDeps at FROM). */ async function makeRepo(): Promise { @@ -979,7 +990,20 @@ describe("runUpgrade: legacy config location", () => { /** Move this repo's config from the root to the legacy location. */ async function useLegacyConfig(): Promise { - await rm(join(repo, "streamctl.config.ts"), { force: true }); + // Tripwire, not coverage — it can only fail from a deliberate edit, and it proves + // nothing about the product. Its job is to turn a silent loss into a loud one. + // + // These four tests cannot detect a `bumpConfigVersion` hardcoded to the legacy path + // (see the comment on `writeConfig`); the root-repo tests above are what can, and + // only while the default fixture stays at the root. Relocating that default is + // survivable in a way that looks fine: a careless flip reds ~28 tests here, but a + // thorough one leaves a handful of path-detail assertions whose obvious fix is to + // update the path — after which the suite is green and the guard is gone. This + // assertion is the step in that sequence that says so, by name. + // + // No `force` on the `rm` either: without it, a moved default no-ops silently here. + expect(existsSync(join(repo, `${CONFIG_FILE}.ts`)), "the default fixture must stay at the root").toBe(true); + await rm(join(repo, `${CONFIG_FILE}.ts`)); await mkdir(join(repo, ".streamctl"), { recursive: true }); await writeFile(join(repo, legacyRel), legacyConfig); } From 40e012a157e0765908ba9b8bb6fc21d9153c41b5 Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Thu, 30 Jul 2026 17:47:16 +0200 Subject: [PATCH 19/28] test(e2e): scaffold and assert the root config in the distribution gate Moves the init leg and the scaffolded-repo builder onto streamctl.config.ts and adds an assertion that no .streamctl/ directory is created, which only this gate can check against the built artifact. Leg 4 deliberately stays on the legacy location and now says why: it is the only artifact-level coverage of the fallback on any package manager. Also fixes the header's leg inventory, which claimed three legs where the script runs six, and records in test/load.test.ts why the eval-once test's positive assertion is free only while the loader must evaluate one file. --- scripts/e2e-dry-run.mjs | 49 ++++++++++++++++++++++++++++++++--------- test/load.test.ts | 6 ++++- 2 files changed, 43 insertions(+), 12 deletions(-) diff --git a/scripts/e2e-dry-run.mjs b/scripts/e2e-dry-run.mjs index 4988119..b6f4369 100644 --- a/scripts/e2e-dry-run.mjs +++ b/scripts/e2e-dry-run.mjs @@ -4,10 +4,10 @@ // `@acme/payload` in test/fixtures/synthetic-payload. // // For the package manager named by E2E_PM (npm, pnpm, yarn or bun) it runs -// three legs on a throwaway repo pinned to that PM via `packageManager`: +// these legs on throwaway repos pinned to that PM via `packageManager`: // -// 1. init --no-install --skip-registry-check. Scaffolds .streamctl/config.ts -// and reports ` install`. No registry traffic. +// 1. init --no-install --skip-registry-check. Scaffolds streamctl.config.ts +// at the repo root and reports ` install`. No registry traffic. // 2. Read-only adoption. "Install" the payload by copying the fixture into // node_modules/@acme/payload, seed-sync, then `check` and `sync --dry-run` // must both exit 0 with a byte-identical tree. @@ -15,9 +15,15 @@ // payload's preset.json and its source, then re-sync with the same binary. // It has to land, which is what proves the CLI is payload-driven rather // than org-coded. +// 4. upgrade against a newer vendored payload, with a chained sync. Runs on +// npm regardless of E2E_PM, and is the one leg that stays on the LEGACY +// config location -- see the comment in runUpgradeLeg. +// 5. --version reports the semver injected at build time. +// 6. --json usage: an unknown command yields a color-free USAGE envelope. // -// Every PM runs all three. Beyond detection and the install-command name (both -// from engine/pm.ts) the behavior is PM-agnostic. +// Every PM runs legs 1-3; 4-6 are PM-independent and run once. Beyond detection +// and the install-command name (both from engine/pm.ts) the behavior is +// PM-agnostic. import { spawnSync } from "node:child_process"; import { createHash } from "node:crypto"; import { cpSync, existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; @@ -91,13 +97,12 @@ function installPayload(work) { return dest; } -/** A throwaway repo pinned to `pm` with a scaffolded `.streamctl/config.ts`. */ +/** A throwaway repo pinned to `pm` with a scaffolded `streamctl.config.ts`. */ function makeRepo(pm) { const work = mkdtempSync(join(tmpdir(), `streamctl-e2e-${pm}-`)); writeFileSync(join(work, "package.json"), `${JSON.stringify({ name: "app", private: true, packageManager: `${pm}@1.0.0` }, null, 2)}\n`); - mkdirSync(join(work, ".streamctl"), { recursive: true }); writeFileSync( - join(work, ".streamctl", "config.ts"), + join(work, "streamctl.config.ts"), `export default { package: "${PKG}", base: "app", version: "${PAYLOAD_VERSION}", profile: "std" };\n`, ); return work; @@ -118,13 +123,22 @@ function runInitSmoke(pm) { fail(`init:${pm}`, res); return; } - if (!existsSync(join(work, ".streamctl", "config.ts"))) { - console.error(`✗ init:${pm}: did not scaffold .streamctl/config.ts`); + if (!existsSync(join(work, "streamctl.config.ts"))) { + console.error(`✗ init:${pm}: did not scaffold streamctl.config.ts`); + process.exitCode = 1; + return; + } + // `existsSync`, never a readFileSync probe: reading a directory throws EISDIR, so a + // try/catch probe would report "absent" for a `.streamctl/` sitting right there. + // A unit test covers this too; here it runs against the BUILT artifact, which is the + // only place a bundling or path-resolution difference between src and dist shows up. + if (existsSync(join(work, ".streamctl"))) { + console.error(`✗ init:${pm}: created a .streamctl/ directory`); process.exitCode = 1; return; } // The scaffolded pin is the PAYLOAD's version, never the CLI's own. - const scaffolded = readFileSync(join(work, ".streamctl", "config.ts"), "utf8"); + const scaffolded = readFileSync(join(work, "streamctl.config.ts"), "utf8"); if (!scaffolded.includes(`version: "${PAYLOAD_VERSION}"`)) { console.error(`✗ init:${pm}: scaffolded pin is not the payload version ${PAYLOAD_VERSION}`); process.exitCode = 1; @@ -258,6 +272,19 @@ function runUpgradeLeg() { }, null, 2)}\n`); // Multi-line config: `upgrade`'s version-pin bump is line-anchored (`version:` on // its own line), unlike the single-line config the other legs scaffold. + // + // LEGACY location on purpose -- do not move this to the root "for consistency" with + // the other legs. Since they moved, this is the ONLY place the `.streamctl/config.*` + // fallback is exercised against the built artifact, on any package manager; move it + // and the feature's headline promise ("a legacy repo behaves exactly as it does + // today") is tested nowhere outside vitest. It is also the right leg to carry that + // cost, because the pin bump is the touch point where a wrong path fails most + // silently. If this leg is ever restructured, the legacy coverage moves with it. + // + // Safe because this leg builds its own repo (`mkdtempSync` above) rather than calling + // `makeRepo`, which now writes the root path: sharing a builder would make the repo + // both-present, root would win, and the assertions below would check a pin bump on a + // file `upgrade` never touched. mkdirSync(join(work, ".streamctl"), { recursive: true }); writeFileSync( join(work, ".streamctl", "config.ts"), diff --git a/test/load.test.ts b/test/load.test.ts index 720d520..fc882fc 100644 --- a/test/load.test.ts +++ b/test/load.test.ts @@ -198,7 +198,11 @@ describe("loadStreamctlConfig", () => { await loadStreamctlConfig(root, { logger: { warn: () => {} } }); // Positive first: it proves the fixture works, so the negative below cannot pass - // because the body silently failed to write. + // because the body silently failed to write. The positive is free here only because + // the loader is *expected* to evaluate one of the two. Extend this to a case where + // neither should be evaluated and the shortcut dies — that needs `resolve.test.ts`'s + // proof phase, which evaluates the module directly rather than relying on the unit + // under test to do it. expect(existsSync(rootSentinel)).toBe(true); expect(existsSync(legacySentinel)).toBe(false); }); From 16341f132115fa8a3a51a38ccf20aa811155480c Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Thu, 30 Jul 2026 17:54:58 +0200 Subject: [PATCH 20/28] docs: point the config location at streamctl.config.ts Sweeps README, docs/ and the two .github/ prompts onto the root default, adds a "Where the config lives" section stating that exactly two locations resolve and that the legacy one is permanent, and drops the .streamctl/** entry from the ESLint ignore example per Q4. The three remaining source comments follow; the six deliberate references in src/ are left alone. Also refreshes docs/release.md's status banner (0.1.0 is on the registry) with notes for the next release, and clarifies in the E2E gate's header that legs 4-6 run once per invocation rather than once per matrix. BREAKING CHANGE: a config at .config/.streamctl/config.ts no longer resolves and now raises NOT_INITIALIZED. Measured against c12 3.3.4: the old `configFile: ".streamctl/config"` spelling made c12 probe `.config/.streamctl/config`, and the new spelling does not. Fix with `git mv .config/.streamctl/config.ts streamctl.config.ts`. Nothing else under .config/ resolved before this release, and nothing does now. --- .github/CONTRIBUTING.md | 3 ++- .github/ISSUE_TEMPLATE/bug-report.yaml | 2 +- README.md | 27 +++++++++++++++++---- docs/adoption.md | 8 +++---- docs/reference.md | 8 +++---- docs/release.md | 33 ++++++++++++++++++++++---- scripts/e2e-dry-run.mjs | 3 ++- src/config/index.ts | 2 +- src/config/types.ts | 2 +- src/report.ts | 2 +- 10 files changed, 66 insertions(+), 24 deletions(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 867c234..ef5a390 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -22,7 +22,8 @@ Because streamctl works entirely on local files (the payload is read from `node_ - The exact command you ran and its full output, ideally with `--json` (e.g. `pnpm streamctl sync --dry-run --json`) - The output of `pnpm streamctl status --json` -- The payload package name and its pinned `version` from `.streamctl/config.ts` +- The payload package name and its pinned `version` from `streamctl.config.ts` (or + `.streamctl/config.ts`, if the repo is on the legacy location) ### Opening an issue diff --git a/.github/ISSUE_TEMPLATE/bug-report.yaml b/.github/ISSUE_TEMPLATE/bug-report.yaml index 4d36247..39554b3 100644 --- a/.github/ISSUE_TEMPLATE/bug-report.yaml +++ b/.github/ISSUE_TEMPLATE/bug-report.yaml @@ -44,7 +44,7 @@ body: id: bug-payload attributes: label: Payload - description: The payload package name and the pinned `version` from `.streamctl/config.ts` (plus any relevant config knobs) + description: The payload package name and the pinned `version` from your streamctl config -- `streamctl.config.ts`, or `.streamctl/config.ts` on the legacy location (plus any relevant config knobs) placeholder: "@your-org/config @ 1.2.3" validations: required: true diff --git a/README.md b/README.md index ce48c22..75ebd66 100644 --- a/README.md +++ b/README.md @@ -67,7 +67,7 @@ pnpm streamctl upgrade # move the pinned version forward, re-sync ### `streamctl init` -Wire a repo to a payload for the first time. Reads the payload's manifest to pick the base preset and auto-detect the profile (the manifest ships the detection probes, so the CLI has no framework knowledge), scaffolds `.streamctl/config.ts` from the payload's own template, adds the devDependencies, runs the install, then runs the first `sync`. +Wire a repo to a payload for the first time. Reads the payload's manifest to pick the base preset and auto-detect the profile (the manifest ships the detection probes, so the CLI has no framework knowledge), scaffolds `streamctl.config.ts` from the payload's own template, adds the devDependencies, runs the install, then runs the first `sync`. | Flag | Type | Effect | | ---- | ---- | ------ | @@ -153,7 +153,7 @@ pnpm streamctl status --outdated --json | jq '.data.files' ### `streamctl upgrade` -The only command that moves the pinned payload version forward. Before it touches anything it snapshots `.streamctl/config.ts`, `package.json` and the lockfile, and any failure along the way (a bad install, an invalid new payload, a conflict) restores all three byte-for-byte. The [reference](docs/reference.md#the-upgrade-transaction) covers the full transaction, including upgrading over a local `file:` override. +The only command that moves the pinned payload version forward. Before it touches anything it snapshots the config file, `package.json` and the lockfile, and any failure along the way (a bad install, an invalid new payload, a conflict) restores all three byte-for-byte. The [reference](docs/reference.md#the-upgrade-transaction) covers the full transaction, including upgrading over a local `file:` override. | Flag | Type | Effect | | ---- | ---- | ------ | @@ -194,7 +194,7 @@ Structured files (`.json*`, `.ya?ml`) get extra safety: composed output is parse ## Configuration -`init` scaffolds `.streamctl/config.ts` from the payload's own template. The payload exports a typed define function (via its `./config` subpath), so the knobs get full editor inference: +`init` scaffolds `streamctl.config.ts` at the repo root, from the payload's own template. The payload exports a typed define function (via its `./config` subpath), so the knobs get full editor inference: ```ts import { defineConfig } from "@your-org/config/config"; @@ -215,6 +215,23 @@ export default defineConfig({ A payload that ships no `config.template.ts` gets a generic fallback that imports the CLI's own `defineStreamctlConfig` from `@sidebase/streamctl` (same fields, minus the typed knobs). +### Where the config lives + +Exactly two locations are read, and no others: + +1. `streamctl.config.ts` at the repo root — the default, and what `init` writes. +2. `.streamctl/config.` — the original location, read **permanently**. It is not + deprecated, there is no warning, and there is no plan to remove it. + +Nothing under `.config/` is read. Both locations accept any extension c12 supports +(`.ts`, `.js`, `.mjs`, `.json`, `.yaml`, …). + +Moving an existing config is a plain `git mv .streamctl/config.ts streamctl.config.ts` +and nothing else — every command behaves identically either way, which +`test/status.command.test.ts` asserts by running the same command against both layouts +and comparing the reports. If both files exist the root one wins and `streamctl` says so +on stderr once; delete the legacy file to silence it. + ## CI setup Add the gate to your pipeline; exit `3` means the tree drifted from the payload: @@ -232,10 +249,10 @@ The probe degrades quietly. If the registry cannot be reached, `check` skips the | Symptom | Likely cause | Fix | | ------- | ------------ | --- | -| `NOT_INITIALIZED` | no `.streamctl/config.ts` | run `streamctl init` first | +| `NOT_INITIALIZED` | no `streamctl.config.ts` (and no legacy `.streamctl/config.ts`) | run `streamctl init` first | | `CONFIG_PKG_MISSING` | the payload package is not installed | `pnpm install` | | `CONFIG_VERSION_MISMATCH` | installed payload version differs from the pinned `version` | `streamctl upgrade` or `pnpm install` | -| `CONFIG_INVALID` | bad `.streamctl/config.ts`, malformed `preset.json`/`package.json`, or an invalid knob | fix the offending file/value (the `details.path` names it) | +| `CONFIG_INVALID` | bad `streamctl.config.ts`, malformed `preset.json`/`package.json`, or an invalid knob | fix the offending file/value (the `details.path` names it) | | exit `2` (`CONFLICTS_PENDING`) | a `full` file you edited, a marker/merge/structural fault, or a dirty owned path | review the plan; `sync --interactive` to confirm, or `--force` to accept | | exit `3` (`DRIFT_DETECTED`) | the working tree drifted from the payload (CI gate) | run `streamctl sync` and commit | | `REGISTRY_AUTH_FAILED` | cannot read the payload from GitHub Packages | check the token's `read:packages` scope | diff --git a/docs/adoption.md b/docs/adoption.md index 6a409ae..5d0bcf7 100644 --- a/docs/adoption.md +++ b/docs/adoption.md @@ -38,7 +38,7 @@ pnpm dlx @sidebase/streamctl init --package @your-org/config --yes `init` detects the Nuxt major from `package.json` and proposes `base: "nuxt-app"` + `profile: "nuxt-4"`. It then: -- writes `.streamctl/config.ts` (the pinned `version` + your knobs), +- writes `streamctl.config.ts` at the repo root (the pinned `version` + your knobs), - scaffolds the `.npmrc` registry block (incl. `always-auth=true`), - adds the `@sidebase/streamctl` + your payload (`@your-org/config`) + `jiti` devDependencies and runs the install (so the preset payload lands on disk), @@ -120,7 +120,7 @@ script is not a semver), so it replaces whatever the repo has — opt out per-ke `versionSyncExclude`. Everything the baseline does not list (`vue`, `tailwindcss`, app deps, your own scripts) is project-owned and never touched. -Disable globally or per-key in `.streamctl/config.ts`: +Disable globally or per-key in `streamctl.config.ts`: ```ts export default { @@ -134,7 +134,7 @@ export default { When a single repo has to hold one pin back, reach for `versionSyncExclude` rather than turning `versionSync` off wholesale, and record why (a comment in -`.streamctl/config.ts` or the PR description). An exclude entry that is not an +`streamctl.config.ts` or the PR description). An exclude entry that is not an active allow-list key is rejected with `CONFIG_INVALID`. ## 5. `upgrade` (moving the pin forward) @@ -149,7 +149,7 @@ pnpm streamctl upgrade --dry-run # resolve target + intended bumps, write nothi It resolves the target first (`NO_NEWER_VERSION` if you are already on the latest, `TARGET_NOT_FOUND` if `--to` names an unpublished version), bumps the -`.streamctl/config.ts` pin and the payload devDep in lockstep, runs the install, +config-file pin and the payload devDep in lockstep, runs the install, then runs `sync`, interactive by default. Review the diff and commit. A `--dry-run` issued before the new presets are installed prints `preview unavailable: presets not installed` instead of a misleading empty plan. diff --git a/docs/reference.md b/docs/reference.md index ceee36b..7f4595b 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -49,7 +49,7 @@ The dirty-tree guard: if a streamctl-owned path has uncommitted tracked edits, ` ## How `init` picks the payload version -The `.streamctl/config.ts` `version` pin is the payload's version, never the CLI's. The two +The `streamctl.config.ts` `version` pin is the payload's version, never the CLI's. The two packages release on their own cadence. `init` resolves the pin in this order, before it writes anything: @@ -100,7 +100,7 @@ When the version reconcile edits `package.json` (non-`--dry-run`) and a lockfile `upgrade` is the only command that moves the pinned version forward. Either it applies in full, or it puts the repo back exactly as it was. -The flow runs in this order. Resolve the target, which is the latest published version or whatever `--to` names. Snapshot the three files a failed run could leave inconsistent: `.streamctl/config.ts`, `package.json`, and the detected package manager's lockfile. Bump the pin and the payload devDependency, leaving the CLI's own version alone. Install, so the new bundled presets land on disk. Finally, preflight and apply the first `sync` against the new version. +The flow runs in this order. Resolve the target, which is the latest published version or whatever `--to` names. Snapshot the three files a failed run could leave inconsistent: the config file (wherever it resolved), `package.json`, and the detected package manager's lockfile. Bump the pin and the payload devDependency, leaving the CLI's own version alone. Install, so the new bundled presets land on disk. Finally, preflight and apply the first `sync` against the new version. The preflight comes for free from `sync` being transactional. It composes and validates the whole batch and writes nothing until the batch is clean, so a non-interactive conflict or an invalid new payload throws before any managed file changes. @@ -141,8 +141,8 @@ Local-tarball adoption skips `init`'s registry probe. If a `pnpm.overrides` or r **Payload content that changes under the same version shows up as an `edit` conflict.** Drift is derived and there is no state file, so sync cannot tell a repacked local tarball apart from a local edit. It blocks and asks for review (`--interactive` or `--force`). Fleet updates are better carried by a version bump and `streamctl upgrade`, where the preflight previews the change for you. -Config changes hit the same wall. Edit a `.streamctl/config.ts` knob that feeds a `full`-strategy render (placeholders, fragment toggles) and the next `sync` reports the render delta as `edit`/`adoption` conflicts and exits `2`. Resolve with `sync --interactive`, `sync --force` or `--only `. `block`/`merge` reconciles are unaffected. A rendered-content baseline would remove this friction, and may show up later. +Config changes hit the same wall. Edit a `streamctl.config.ts` knob that feeds a `full`-strategy render (placeholders, fragment toggles) and the next `sync` reports the render delta as `edit`/`adoption` conflicts and exits `2`. Resolve with `sync --interactive`, `sync --force` or `--only `. `block`/`merge` reconciles are unaffected. A rendered-content baseline would remove this friction, and may show up later. -**The shipped ESLint wrapper is not a drop-in for a complex repo.** A bare `createStreamctlEslint()` lints everything, scratch and artifact directories included. Real adopters chain their own ignores onto it: `createStreamctlEslint().append({ ignores: ["scratch/**", ".streamctl/**", /* ... */] })`. +**The shipped ESLint wrapper is not a drop-in for a complex repo.** A bare `createStreamctlEslint()` lints everything, scratch and artifact directories included. Real adopters chain their own ignores onto it: `createStreamctlEslint().append({ ignores: ["scratch/**", /* ... */] })`. The Node `engines` floor is inherited. `^22.22.2 || ^24.15.0 || >=26.0.0` copies `write-file-atomic@8`'s own `engines` requirement verbatim, because that atomic-write dependency is what sets the real floor. Relaxing streamctl's range below it would just move the install warning down to the dependency. diff --git a/docs/release.md b/docs/release.md index 9b0142f..849c9f7 100644 --- a/docs/release.md +++ b/docs/release.md @@ -1,9 +1,9 @@ # Release runbook (`@sidebase/streamctl`) -> **Status: PARKED.** Nothing is published yet. The `Release` workflow -> (`.github/workflows/release.yml`) is a `workflow_dispatch`-only draft gated -> behind the protected `release` environment. It cannot publish anything until a -> maintainer completes the one-time setup below and dispatches it by hand. +> **Status: 0.1.0 is on the registry.** The `Release` workflow +> (`.github/workflows/release.yml`) stays `workflow_dispatch`-only and gated behind +> the protected `release` environment, so every publish is a deliberate manual +> dispatch by a maintainer who has completed the one-time setup below. `streamctl` publishes to the public npm registry under the `@sidebase` scope. It is an intentionally ESM-only package; the published tarball ships only `dist/`. @@ -19,7 +19,7 @@ What couples the CLI to a payload is the payload manifest's integer - If the running CLI does not support a payload's `schemaVersion`, the user gets a dedicated "payload requires a newer/older streamctl" error rather than a generic `CONFIG_INVALID`. The loader leaves room for per-version migrations later. -- The `.streamctl/config.ts` `version` pin governs the payload package only, and +- The config file's `version` pin governs the payload package only, and `CONFIG_VERSION_MISMATCH` compares the installed payload against that pin. `upgrade` moves the payload pin and its devDep, and leaves the CLI version alone. @@ -66,6 +66,29 @@ approve the environment gate. The workflow: After the run, verify the published tarball on npm, the `vX.Y.Z` tag, and the generated GitHub Release notes. +## Notes for the next release + +Include these in the release notes; the rest is generated from commit subjects. + +- **The config file's default location moved** to `streamctl.config.ts` at the repo + root. `init` writes it there. +- **`.streamctl/config.*` keeps working, permanently.** Not deprecated, no warning, + no removal planned. Existing repos need to do nothing. A repo that *does* move its + config needs this CLI version or newer. +- **One breaking edge:** a config at `.config/.streamctl/config.ts` resolved before + this release and does not now — it raises `NOT_INITIALIZED`. Measured against + c12 3.3.4: the old `configFile: ".streamctl/config"` spelling made c12 probe + `.config/.streamctl/config`, and the new spelling does not. The form is + undocumented and nested, so realistically nobody is on it, but the fix is one + command: + + ```sh + git mv .config/.streamctl/config.ts streamctl.config.ts + ``` + + Nothing else under `.config/` ever resolved, and nothing does now. +- Minor bump: new default, no removals. + Use Conventional Commit subjects (and `!` / `BREAKING CHANGE:` for anything that moves the `--json` envelope, exit codes, or the manifest `schemaVersion`) so the history reads clearly for consumers. diff --git a/scripts/e2e-dry-run.mjs b/scripts/e2e-dry-run.mjs index b6f4369..efe113c 100644 --- a/scripts/e2e-dry-run.mjs +++ b/scripts/e2e-dry-run.mjs @@ -21,7 +21,8 @@ // 5. --version reports the semver injected at build time. // 6. --json usage: an unknown command yields a color-free USAGE envelope. // -// Every PM runs legs 1-3; 4-6 are PM-independent and run once. Beyond detection +// Every PM runs legs 1-3; 4-6 run once per invocation regardless of E2E_PM (so a +// four-PM CI matrix runs each of them four times). Beyond detection // and the install-command name (both from engine/pm.ts) the behavior is // PM-agnostic. import { spawnSync } from "node:child_process"; diff --git a/src/config/index.ts b/src/config/index.ts index a95063e..19c3c0b 100644 --- a/src/config/index.ts +++ b/src/config/index.ts @@ -3,7 +3,7 @@ import type { StreamctlConfig } from "./types"; export * from "./types"; /** - * Identity helper for `.streamctl/config.ts`: gives editor inference and flags + * Identity helper for `streamctl.config.ts`: gives editor inference and flags * unknown keys while returning the config unchanged. Generic over `T` so a payload * can build its own typed wrapper and keep its narrower field types. */ diff --git a/src/config/types.ts b/src/config/types.ts index f310431..228eaeb 100644 --- a/src/config/types.ts +++ b/src/config/types.ts @@ -48,7 +48,7 @@ export interface ManagedFile { shadowedBy?: string[]; } -/** The per-repo manifest owned by a consuming repo (`.streamctl/config.ts`). */ +/** The per-repo manifest owned by a consuming repo (`streamctl.config.ts`). */ export interface StreamctlConfig { /** The CLI ships no default. */ package: string; diff --git a/src/report.ts b/src/report.ts index ae95b7e..3e4f839 100644 --- a/src/report.ts +++ b/src/report.ts @@ -231,7 +231,7 @@ export function formatCheck(data: CheckResult, style: Style): string { } export function formatInit(data: InitResult, style: Style): string { - // `version` is the payload pin (from `.streamctl/config.ts`); the CLI's own + // `version` is the payload pin (from the repo's streamctl config); the CLI's own // release gets its own line so the two never read as one number. const head = [ kvLine("base", data.base, style), From 2b10b20aeabdee89dd812f080296315e9e0c4d94 Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Thu, 30 Jul 2026 17:56:48 +0200 Subject: [PATCH 21/28] docs: record the measured .config/ probe results The block's claims about what c12 would do under each spelling were reasoned about for two gates. The table replaces the argument with the measurement, including the counterfactual: under the new spelling raw c12 does reach two of the three .config/ shapes, so the existence probe is the only reason it never gets the chance. Also notes in the release runbook that the broken shape only ever worked on c12 >= 3.2.0, without conditioning the migration step on it. --- docs/release.md | 6 ++++++ test/resolve.test.ts | 16 ++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/docs/release.md b/docs/release.md index 849c9f7..f07dff9 100644 --- a/docs/release.md +++ b/docs/release.md @@ -87,6 +87,12 @@ Include these in the release notes; the rest is generated from commit subjects. ``` Nothing else under `.config/` ever resolved, and nothing does now. + + The affected population is narrower than it reads: below c12 3.2.0 there is no + `_configFile`, so the old loader raised `NOT_INITIALIZED` from any location. A + repo on this layout was only ever working if its tree resolved c12 >= 3.2.0. + The `git mv` is worth doing either way, so the instruction above is not + conditional on that. - Minor bump: new default, no removals. Use Conventional Commit subjects (and `!` / `BREAKING CHANGE:` for anything that diff --git a/test/resolve.test.ts b/test/resolve.test.ts index 753783b..8e94283 100644 --- a/test/resolve.test.ts +++ b/test/resolve.test.ts @@ -235,6 +235,22 @@ describe("resolveConfigFile", () => { * unreachable. So there is no rejection code to review, and nothing visibly breaks if * a refactor undoes it. These tests are the only guard. A failure here means the * probe-before-load ordering was lost; do not "fix" it by re-admitting `.config/`. + * + * Measured against c12 3.3.4 — `loadConfig` alone, no probe, `_configFile`: + * + * ``` + * fixture configFile: ".streamctl/config" configFile: "streamctl.config" + * .config/.streamctl/config.ts .config/.streamctl/config.ts (none) + * .config/streamctl.ts (none) .config/streamctl.ts + * .config/streamctl.config.ts (none) .config/streamctl.config.ts + * ``` + * + * The right-hand column is the point: under the spelling this feature adopted, c12 + * would in fact reach into `.config/` for two of the three, and the existence probe is the + * only reason it never gets the chance. Left column, top row, is the one real + * behavior change in the feature — see that test below. Both were reasoned about for + * two gates before being measured; the numbers are here so the next reader inherits + * a measurement rather than the argument. */ describe(".config/ is deliberately not supported", () => { it("does not resolve .config/streamctl.ts", async () => { From 1d67ab1381c0030a01a93a4c6e13daac9e3e0e06 Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Thu, 30 Jul 2026 18:07:05 +0200 Subject: [PATCH 22/28] feat(config): warn when one location holds two config extensions c12 reads the first supported extension in its own order, so streamctl.config.js silently beats streamctl.config.ts -- the reverse of what most people expect. The probe now collects every match at a location instead of stopping at the first, and names the shadowed siblings alongside the file actually read. Applies at the legacy location too; the losing location is ignored wholesale, so a collision inside it is not reported. The winner cannot appear in its own shadow list: it is matches[0] and the candidates are one spelling crossed with twelve distinct extensions. No "skip the resolved file" filter, because it could never fire and would read as protection that is not there -- the precondition it would appear to guard is candidate distinctness, which is now pinned by its own test. Cost note: the probe no longer short-circuits, so a run makes 24 stats rather than as few as 2. Updated in both places that stated the old number. Also corrects "24 existsSync calls" to statSync().isFile() in 70_risks.md and 90_questions.md. That is the fourth and fifth in the same family -- 20_architecture.md, 40_data_model.md and P03-T04's AC 50 were the others -- all from specs written before the implementation chose isFile. If a sixth turns up, it is this pattern, not a local typo. --- README.md | 7 +-- docs/release.md | 2 +- src/config/resolve.ts | 49 ++++++++++++++---- test/init.test.ts | 2 +- test/resolve.test.ts | 113 +++++++++++++++++++++++++++++++++++++++--- 5 files changed, 152 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 75ebd66..b474f76 100644 --- a/README.md +++ b/README.md @@ -227,9 +227,10 @@ Nothing under `.config/` is read. Both locations accept any extension c12 suppor (`.ts`, `.js`, `.mjs`, `.json`, `.yaml`, …). Moving an existing config is a plain `git mv .streamctl/config.ts streamctl.config.ts` -and nothing else — every command behaves identically either way, which -`test/status.command.test.ts` asserts by running the same command against both layouts -and comparing the reports. If both files exist the root one wins and `streamctl` says so +and nothing else — every command behaves identically either way. `status` is the case +held to that by test: `test/status.command.test.ts` runs it against one repo in both +layouts and asserts the two reports are equal, down to the absence of any trace of which +file was read. If both files exist the root one wins and `streamctl` says so on stderr once; delete the legacy file to silence it. ## CI setup diff --git a/docs/release.md b/docs/release.md index f07dff9..08e2cff 100644 --- a/docs/release.md +++ b/docs/release.md @@ -86,7 +86,7 @@ Include these in the release notes; the rest is generated from commit subjects. git mv .config/.streamctl/config.ts streamctl.config.ts ``` - Nothing else under `.config/` ever resolved, and nothing does now. + Nothing else under `.config/` is read by streamctl, before or after this release. The affected population is narrower than it reads: below c12 3.2.0 there is no `_configFile`, so the old loader raised `NOT_INITIALIZED` from any location. A diff --git a/src/config/resolve.ts b/src/config/resolve.ts index 50564e7..27ec40b 100644 --- a/src/config/resolve.ts +++ b/src/config/resolve.ts @@ -50,14 +50,25 @@ function isFile(abs: string): boolean { } } -function probe(cwd: string, spelling: string): string | null { +/** + * Every existing file for a spelling, in c12's precedence order — the first entry is + * the one c12 will load. + * + * Walks the whole list rather than short-circuiting, which is what makes the shadow + * warning possible: stopping at the first hit cannot see that a second exists. The cost + * is fixed at 24 stats per run (12 extensions × 2 locations) instead of as few as 2, + * all against paths the OS has cached, and it is noise beside jiti compiling a TS + * config. + */ +function probeAll(cwd: string, spelling: string): string[] { + const matches: string[] = []; for (const candidate of configCandidates(spelling)) { const abs = resolve(cwd, candidate); if (isFile(abs)) { - return abs; + matches.push(abs); } } - return null; + return matches; } /** @@ -79,21 +90,41 @@ function probe(cwd: string, spelling: string): string | null { * not a breaking change (`50_api.md`). */ export async function resolveConfigFile(cwd: string, logger?: Logger): Promise { - const rootAbs = probe(cwd, CONFIG_FILE); + const rootMatches = probeAll(cwd, CONFIG_FILE); // Probed even on a root hit: it is the only signal for the both-present warning. - const legacyAbs = probe(cwd, LEGACY_CONFIG_FILE); - const abs = rootAbs ?? legacyAbs; - if (abs === null) { + const legacyMatches = probeAll(cwd, LEGACY_CONFIG_FILE); + + // The winning location, entire. Only this one can shadow: the other is ignored + // wholesale, so reporting a collision inside it would be noise about files that make + // no difference either way. + const [rootAbs] = rootMatches; + const [legacyAbs] = legacyMatches; + const [abs, ...shadowed] = rootAbs === undefined ? legacyMatches : rootMatches; + if (abs === undefined) { return null; } const toRel = (path: string): string => relative(cwd, path).split(sep).join("/"); - if (rootAbs !== null && legacyAbs !== null) { + // Shadow first, then cross-location: a shadow is known as soon as one location has + // been probed, while the cross warning needs both. Pinned by test so the order stays + // predictable in CI logs rather than following whatever the code happens to do. + // + // `abs` cannot appear in `shadowed`: it is `matches[0]` and the candidates are one + // spelling crossed with twelve distinct extensions, so a file can match at most once. + // That is why there is no "skip the resolved file" check here — it could never fire. + // The precondition is candidate distinctness, which `resolve.test.ts` pins directly. + if (shadowed.length > 0) { + logger?.warn( + `streamctl: ${shadowed.map(toRel).join(" and ")} ${shadowed.length > 1 ? "are" : "is"} shadowed by ${toRel(abs)}; ${toRel(abs)} is the one being read.`, + ); + } + + if (rootAbs !== undefined && legacyAbs !== undefined) { logger?.warn( `streamctl: both ${toRel(rootAbs)} and ${toRel(legacyAbs)} exist; using ${toRel(rootAbs)} and ignoring ${toRel(legacyAbs)}.`, ); } - return { abs, rel: toRel(abs), source: rootAbs !== null ? "root" : "legacy" }; + return { abs, rel: toRel(abs), source: rootAbs === undefined ? "legacy" : "root" }; } diff --git a/test/init.test.ts b/test/init.test.ts index 64f6e2a..3cb2ef3 100644 --- a/test/init.test.ts +++ b/test/init.test.ts @@ -238,7 +238,7 @@ describe("runInit", () => { it("checks for a repo before checking for a config", async () => { // A config with no `package.json` must still fail NOT_A_REPO. The only other // NOT_A_REPO test has neither file, so it cannot see the ordering — and the config - // guard is now an awaited resolver call doing up to 24 stats, which invites being + // guard is now an awaited resolver call doing 24 stats every time, which invites being // hoisted above the cheap synchronous probe. await writeFile(join(repo, "streamctl.config.ts"), "export default {}\n"); diff --git a/test/resolve.test.ts b/test/resolve.test.ts index 8e94283..fc0f832 100644 --- a/test/resolve.test.ts +++ b/test/resolve.test.ts @@ -191,8 +191,10 @@ describe("resolveConfigFile", () => { const location = await resolveConfigFile(root, logger); expect(location?.rel).toBe("streamctl.config.js"); - // Same-location shadowing is not the both-present case. - expect(warnings).toEqual([]); + // One warning, and it is the shadow one — same-location shadowing is not the + // both-present case, which needs a config at each of the two locations. + expect(warnings).toHaveLength(1); + expect(warnings[0]).not.toContain("both"); }); it("builds one candidate per c12 extension, in c12's order", async () => { @@ -202,6 +204,103 @@ describe("resolveConfigFile", () => { expect(configCandidates(CONFIG_FILE)).toEqual(SUPPORTED_EXTENSIONS.map(ext => `streamctl.config${ext}`)); expect(configCandidates(LEGACY_CONFIG_FILE)).toHaveLength(SUPPORTED_EXTENSIONS.length); }); + + it("builds distinct candidates", () => { + // The precondition the shadow warning rests on. The winner is `matches[0]` and the + // shadowed list is everything after it, so the winner can only appear in its own + // shadow list if two candidates are the same spelling — which is the one way the + // resolver could tell a user to delete the file it just chose. `SUPPORTED_EXTENSIONS` + // is c12's, so this is a check on a dependency, not on us. + const candidates = configCandidates(CONFIG_FILE); + expect(new Set(candidates).size).toBe(candidates.length); + }); + }); + + describe("same-location extension shadowing", () => { + it("warns and reads the .js when both extensions exist at the root", async () => { + await writeRootConfig(".ts"); + await writeRootConfig(".js"); + + const location = await resolveConfigFile(root, logger); + + expect(location?.rel).toBe("streamctl.config.js"); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain("streamctl.config.ts is shadowed by streamctl.config.js"); + expect(warnings[0]).toContain("streamctl.config.js is the one being read"); + }); + + it("warns at the legacy location too", async () => { + await writeLegacyConfig(".ts"); + await writeLegacyConfig(".js"); + + const location = await resolveConfigFile(root, logger); + + expect(location?.rel).toBe(".streamctl/config.js"); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain(".streamctl/config.ts is shadowed by .streamctl/config.js"); + }); + + it("names every shadowed sibling", async () => { + await writeRootConfig(".ts"); + await writeRootConfig(".js"); + await writeRootConfig(".mjs"); + + await resolveConfigFile(root, logger); + + const [warning] = warnings; + expect(warning).toContain("streamctl.config.ts"); + expect(warning).toContain("streamctl.config.mjs"); + // Never the winner in its own shadow list: this is the message that would tell a + // user to delete the only config they have. + expect(warning).toContain("are shadowed by streamctl.config.js"); + expect(warning.slice(0, warning.indexOf("are shadowed by"))).not.toContain("streamctl.config.js"); + }); + + it("says nothing when one extension exists", async () => { + await writeRootConfig(".ts"); + + await resolveConfigFile(root, logger); + + expect(warnings).toEqual([]); + }); + + it("ignores shadowing at the losing location", async () => { + // The legacy location is ignored wholesale when a root config exists, so a + // collision inside it changes nothing and warning about it would be noise. + await writeRootConfig(".ts"); + await writeLegacyConfig(".ts"); + await writeLegacyConfig(".js"); + + await resolveConfigFile(root, logger); + + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain("both"); + }); + + it("emits the shadow warning before the cross-location one", async () => { + // A shadow is known once one location has been probed; the cross warning needs + // both. Pinned so CI output stays predictable rather than tracking probe order. + await writeRootConfig(".ts"); + await writeRootConfig(".js"); + await writeLegacyConfig(".ts"); + + await resolveConfigFile(root, logger); + + expect(warnings).toHaveLength(2); + expect(warnings[0]).toContain("is shadowed by"); + expect(warnings[1]).toContain("both streamctl.config.js and .streamctl/config.ts"); + }); + + it("emits nothing when no logger is passed", async () => { + const stderr = captureStderr(); + await writeRootConfig(".ts"); + await writeRootConfig(".js"); + + await resolveConfigFile(root); + + expect(warnings).toEqual([]); + expect(stderr.join("")).toBe(""); + }); }); describe("directory shaped like a config", () => { @@ -246,11 +345,11 @@ describe("resolveConfigFile", () => { * ``` * * The right-hand column is the point: under the spelling this feature adopted, c12 - * would in fact reach into `.config/` for two of the three, and the existence probe is the - * only reason it never gets the chance. Left column, top row, is the one real - * behavior change in the feature — see that test below. Both were reasoned about for - * two gates before being measured; the numbers are here so the next reader inherits - * a measurement rather than the argument. + * would in fact reach into `.config/` for two of the three, and the probe is the only + * reason it never gets the chance. Left column, top row, is the one real behavior + * change in the feature — pinned by `it("does not resolve .config/.streamctl/config.ts")` + * below. Both were reasoned about for two gates before being measured; the numbers are + * here so the next reader inherits a measurement rather than the argument. */ describe(".config/ is deliberately not supported", () => { it("does not resolve .config/streamctl.ts", async () => { From bd8db99458f0147b41995f97f8722e8e94a98e37 Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Thu, 30 Jul 2026 18:20:01 +0200 Subject: [PATCH 23/28] docs: document the extension-shadowing warning It fires on repos that changed nothing -- a streamctl.config.js sitting beside a streamctl.config.ts since before this release starts warning on every command -- so it belongs in the release notes and in the README section that already sets up the trap by listing the accepted extensions without saying .js beats .ts. Both use the word the warning itself prints, so a user who greps the docs for text from the message finds it. Also pins two acceptance criteria that were true but unasserted: the shadow warning is stderr-only, cloned from the cross-location test rather than inherited from a shared code path, and candidate distinctness now covers both spellings, since the warning fires at either location. --- README.md | 7 +++++++ docs/release.md | 5 +++++ test/ambiguity.command.test.ts | 31 +++++++++++++++++++++++++++++++ test/resolve.test.ts | 8 ++++++-- 4 files changed, 49 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index b474f76..91f44d7 100644 --- a/README.md +++ b/README.md @@ -226,6 +226,13 @@ Exactly two locations are read, and no others: Nothing under `.config/` is read. Both locations accept any extension c12 supports (`.ts`, `.js`, `.mjs`, `.json`, `.yaml`, …). +If one location holds two of them, **`.js` wins over `.ts`** — that is c12's own +precedence order, and it is the reverse of what most people expect, so `streamctl` +warns on stderr that the `.ts` is *shadowed by* the `.js`, naming the file it actually +read. Only the location in use is checked: +if a root config exists, a collision inside `.streamctl/` is not reported, because +nothing there is read either way. + Moving an existing config is a plain `git mv .streamctl/config.ts streamctl.config.ts` and nothing else — every command behaves identically either way. `status` is the case held to that by test: `test/status.command.test.ts` runs it against one repo in both diff --git a/docs/release.md b/docs/release.md index 08e2cff..16c35d3 100644 --- a/docs/release.md +++ b/docs/release.md @@ -93,6 +93,11 @@ Include these in the release notes; the rest is generated from commit subjects. repo on this layout was only ever working if its tree resolved c12 >= 3.2.0. The `git mv` is worth doing either way, so the instruction above is not conditional on that. +- **New warning:** two extensions of the same config at one location + (`streamctl.config.js` next to `streamctl.config.ts`) now warn on stderr that one is + *shadowed by* the other, naming the one being read. c12's order puts `.js` ahead of + `.ts`, which surprises most people. **Nothing is read differently than before** — this + is a new diagnostic, not new behaviour, so a repo that sees it needs no migration. - Minor bump: new default, no removals. Use Conventional Commit subjects (and `!` / `BREAKING CHANGE:` for anything that diff --git a/test/ambiguity.command.test.ts b/test/ambiguity.command.test.ts index 603fc60..008b4aa 100644 --- a/test/ambiguity.command.test.ts +++ b/test/ambiguity.command.test.ts @@ -100,3 +100,34 @@ describe("both config locations present", () => { expect(lines).toHaveLength(1); }); }); + +describe("two extensions at one location", () => { + // The same channel contract as the block above, asserted directly rather than + // inherited. Both warnings route through `logger?.warn`, so the shadow warning is + // stderr-only for the same reason the cross-location one is — but "true because it + // shares a code path" is an argument, and an argument is what this file exists to + // replace. If the two ever diverge, this is what notices. + beforeEach(async () => { + await rm(join(repo, ".streamctl"), { recursive: true, force: true }); + await writeFile(join(repo, "streamctl.config.js"), config(VERSION)); + }); + + it("warns on stderr and keeps the --json envelope clean", async () => { + const stderr = captureStderr(); + + await statusCommand.run?.({ args: { json: true } } as unknown as StatusArgs); + + expect(process.exitCode).toBe(0); + + const envelope = JSON.parse(stdout.join("")) as { ok: boolean; data: { warnings?: unknown[] } }; + expect(envelope.ok).toBe(true); + expect(stdout.join("")).not.toContain("streamctl:"); + // Not the report's `warnings[]` either, which is a separate channel that does reach + // the envelope -- `collectShadowWarnings` writes there, and this deliberately does not. + expect(JSON.stringify(envelope.data.warnings ?? [])).not.toContain("shadowed"); + + const lines = stderr.join("").split("\n").filter(line => line.length > 0); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain("streamctl.config.ts is shadowed by streamctl.config.js"); + }); +}); diff --git a/test/resolve.test.ts b/test/resolve.test.ts index fc0f832..e38d67b 100644 --- a/test/resolve.test.ts +++ b/test/resolve.test.ts @@ -211,8 +211,12 @@ describe("resolveConfigFile", () => { // shadow list if two candidates are the same spelling — which is the one way the // resolver could tell a user to delete the file it just chose. `SUPPORTED_EXTENSIONS` // is c12's, so this is a check on a dependency, not on us. - const candidates = configCandidates(CONFIG_FILE); - expect(new Set(candidates).size).toBe(candidates.length); + // Both spellings: the warning fires at either location, so a distinctness claim + // about only one is narrower than the property it backs. + for (const spelling of [CONFIG_FILE, LEGACY_CONFIG_FILE]) { + const candidates = configCandidates(spelling); + expect(new Set(candidates).size, spelling).toBe(candidates.length); + } }); }); From ba448a53ce38c8e003b56e8486f19f22daf97249 Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Thu, 30 Jul 2026 18:43:36 +0200 Subject: [PATCH 24/28] test: assert the whole envelope, not a guessed field status has no warnings[] -- that is SyncResult's, reaching the envelopes of sync and check -- so probing data.warnings guessed a shape this command does not have and missed any leak landing elsewhere. Asserting the serialized envelope is a strict superset and needs no knowledge of the result shape. It also covers what the prefix check cannot, since collectShadowWarnings writes without the streamctl: prefix. --- README.md | 5 ++--- test/ambiguity.command.test.ts | 12 ++++++++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 91f44d7..815efd4 100644 --- a/README.md +++ b/README.md @@ -229,9 +229,8 @@ Nothing under `.config/` is read. Both locations accept any extension c12 suppor If one location holds two of them, **`.js` wins over `.ts`** — that is c12's own precedence order, and it is the reverse of what most people expect, so `streamctl` warns on stderr that the `.ts` is *shadowed by* the `.js`, naming the file it actually -read. Only the location in use is checked: -if a root config exists, a collision inside `.streamctl/` is not reported, because -nothing there is read either way. +read. Only the location in use is checked: if a root config exists, a collision inside +`.streamctl/` is not reported, because nothing there is read either way. Moving an existing config is a plain `git mv .streamctl/config.ts streamctl.config.ts` and nothing else — every command behaves identically either way. `status` is the case diff --git a/test/ambiguity.command.test.ts b/test/ambiguity.command.test.ts index 008b4aa..978db07 100644 --- a/test/ambiguity.command.test.ts +++ b/test/ambiguity.command.test.ts @@ -119,12 +119,16 @@ describe("two extensions at one location", () => { expect(process.exitCode).toBe(0); - const envelope = JSON.parse(stdout.join("")) as { ok: boolean; data: { warnings?: unknown[] } }; + const envelope = JSON.parse(stdout.join("")) as { ok: boolean }; expect(envelope.ok).toBe(true); + // Whole envelope, not a named field: `status` has no `warnings[]` -- that is + // `SyncResult`'s, reaching the envelopes of `sync` and `check` -- so probing + // `data.warnings` here would guess a shape this command does not have and miss a leak + // landing anywhere else. This also catches what the prefix check below cannot: + // `collectShadowWarnings` writes *without* the `streamctl: ` prefix, so a shadow + // notice routed into the report would carry no prefix to match on. + expect(stdout.join("")).not.toContain("shadowed"); expect(stdout.join("")).not.toContain("streamctl:"); - // Not the report's `warnings[]` either, which is a separate channel that does reach - // the envelope -- `collectShadowWarnings` writes there, and this deliberately does not. - expect(JSON.stringify(envelope.data.warnings ?? [])).not.toContain("shadowed"); const lines = stderr.join("").split("\n").filter(line => line.length > 0); expect(lines).toHaveLength(1); From da61184591f1bf90a4f1c7b179368f899417d64b Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Thu, 30 Jul 2026 19:15:06 +0200 Subject: [PATCH 25/28] test: correct what the tripwire comment claims about relocation It promised an intermediate signal that does not exist -- "a handful of path-detail assertions" going red, which a reader would take as a moment they might notice. Measured at the Phase 3 gate: a faithful relocation with the assertion removed is 57/57 green on the first run, and a full P02-T02 revert on top is also 57/57. The path details are part of doing the move correctly, not a warning. A comment that understates the hazard invites the "I would have noticed" reasoning the measurement disproves. --- test/upgrade.test.ts | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/test/upgrade.test.ts b/test/upgrade.test.ts index ef482ba..de08784 100644 --- a/test/upgrade.test.ts +++ b/test/upgrade.test.ts @@ -995,11 +995,17 @@ describe("runUpgrade: legacy config location", () => { // // These four tests cannot detect a `bumpConfigVersion` hardcoded to the legacy path // (see the comment on `writeConfig`); the root-repo tests above are what can, and - // only while the default fixture stays at the root. Relocating that default is - // survivable in a way that looks fine: a careless flip reds ~28 tests here, but a - // thorough one leaves a handful of path-detail assertions whose obvious fix is to - // update the path — after which the suite is green and the guard is gone. This - // assertion is the step in that sequence that says so, by name. + // only while the default fixture stays at the root. + // + // Do not expect to notice if you move it. Measured: a *faithful* relocation — this + // helper, `readConfig`, `hashSnapshot`, the `NOT_INITIALIZED` teardown, the `pkgDir` + // blocks and the three path-detail expectations, all handled properly — is 57/57 + // green on the first run with this assertion removed, and a full `P02-T02` revert on + // top of that is *also* 57/57 green. There is no red stage to catch it at: the path + // details are part of doing the relocation correctly, not a warning that something + // is wrong. This assertion is the only thing between a competent fixture move and + // total loss of that guard, which is why it earns its place despite proving nothing + // about the product. // // No `force` on the `rm` either: without it, a moved default no-ops silently here. expect(existsSync(join(repo, `${CONFIG_FILE}.ts`)), "the default fixture must stay at the root").toBe(true); From 484e32202802c97e5bcef26a27d659224486f7dc Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Thu, 30 Jul 2026 19:18:42 +0200 Subject: [PATCH 26/28] docs: name the property that keeps shadow selection safe The absent "skip the resolved file" filter was justified by candidate distinctness, which is the weaker of the two reasons the code is correct. Under symlink or hardlink aliasing two distinct spellings are one file, and the winner still never lands in its own shadow list -- because it is matches[0] and excluded by position, not by identity. A maintainer switching to identity- or set-based selection would break that while believing the invariant was preserved, since the distinctness test would still pass. It pins the precondition for message quality, not this property. --- src/config/resolve.ts | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/config/resolve.ts b/src/config/resolve.ts index 27ec40b..d656e11 100644 --- a/src/config/resolve.ts +++ b/src/config/resolve.ts @@ -110,10 +110,22 @@ export async function resolveConfigFile(cwd: string, logger?: Logger): Promise m !== abs)`) breaks this while + // looking like it preserves the invariant, because the surviving distinctness test + // would still pass. That test pins the *precondition* the message quality rests on — + // duplicate spellings would name a file as its own shadow — not this property. if (shadowed.length > 0) { logger?.warn( `streamctl: ${shadowed.map(toRel).join(" and ")} ${shadowed.length > 1 ? "are" : "is"} shadowed by ${toRel(abs)}; ${toRel(abs)} is the one being read.`, From 45ab1b05b60cdc9a9b42adbd276a9a5de98913a7 Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Thu, 30 Jul 2026 19:23:15 +0200 Subject: [PATCH 27/28] test: pin the shadow warning as descriptive, not prescriptive statSync follows symlinks, so a streamctl.config.js symlinked to streamctl.config.ts passes isFile twice and the warning names one file as shadowing itself under two paths. Measured, not hypothetical. Positional selection removes self-comparison but not this: an alias can appear in the shadowed list under a different path. What keeps that cosmetic is the wording -- "Y is the one being read" states a fact, where the shadowedBy precedent says "port and delete", which in this state would tell someone to delete the file their config lives in and leave a dangling symlink. Nothing pinned that. The test asserts the message carries no imperative verb, rather than matching a fixed sentence that would red on any rewording. The resolver comment now names both properties instead of implying positional selection covers the aliasing case too. --- src/config/resolve.ts | 29 ++++++++++++++++++----------- test/resolve.test.ts | 41 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 57 insertions(+), 13 deletions(-) diff --git a/src/config/resolve.ts b/src/config/resolve.ts index d656e11..bd1767d 100644 --- a/src/config/resolve.ts +++ b/src/config/resolve.ts @@ -114,18 +114,25 @@ export async function resolveConfigFile(cwd: string, logger?: Logger): Promise m !== abs)`) breaks this while - // looking like it preserves the invariant, because the surviving distinctness test - // would still pass. That test pins the *precondition* the message quality rests on — - // duplicate spellings would name a file as its own shadow — not this property. + // 1. **Positional selection** removes *self*-comparison. `abs` is `matches[0]` and + // `shadowed` is everything after it, so the winner is excluded by where it sits in + // the list, whatever it points at. This is stronger than candidate distinctness, + // which is a claim about strings while the hazard is about files. Keep selecting by + // position: switching to identity- or set-based selection (dedupe by realpath, a + // `Set`, `filter(m => m !== abs)`) breaks this while looking like it preserves the + // invariant, because the distinctness test would still pass. + // + // 2. **Descriptive wording** is what makes *alias*-comparison harmless — and position + // does not help there. `statSync` follows symlinks, so a `streamctl.config.js` + // symlinked to `streamctl.config.ts` passes `isFile` twice and the warning names + // one file as shadowing itself under two paths. Measured, not hypothetical. It is + // only cosmetic because the message *describes* ("Y is the one being read") rather + // than *instructs*: the `shadowedBy` precedent says "port and delete", which in + // this state would tell someone to delete the file their config actually lives in. + // Pinned by the symlink test in `resolve.test.ts`, which asserts the message + // carries no imperative. Do not add one. if (shadowed.length > 0) { logger?.warn( `streamctl: ${shadowed.map(toRel).join(" and ")} ${shadowed.length > 1 ? "are" : "is"} shadowed by ${toRel(abs)}; ${toRel(abs)} is the one being read.`, diff --git a/test/resolve.test.ts b/test/resolve.test.ts index e38d67b..efafd54 100644 --- a/test/resolve.test.ts +++ b/test/resolve.test.ts @@ -1,5 +1,5 @@ -import { chmodSync, existsSync, mkdirSync, mkdtempSync, realpathSync, rmSync, statSync } from "node:fs"; -import { chmod, mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { chmodSync, existsSync, mkdirSync, mkdtempSync, realpathSync, rmSync, statSync, symlinkSync, writeFileSync } from "node:fs"; +import { chmod, mkdir, mkdtemp, readFile, rm, symlink, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { dirname, isAbsolute, join, resolve } from "node:path"; import { pathToFileURL } from "node:url"; @@ -32,6 +32,22 @@ function chmodCanRevokeTraversal(): boolean { const canRevokeTraversal = chmodCanRevokeTraversal(); +/** Can this platform/user create symlinks at all? Windows often cannot. */ +function symlinksSupported(): boolean { + const dir = mkdtempSync(join(tmpdir(), "streamctl-symprobe-")); + try { + writeFileSync(join(dir, "t"), "x"); + symlinkSync(join(dir, "t"), join(dir, "l")); + return true; + } catch { + return false; + } finally { + rmSync(dir, { recursive: true, force: true }); + } +} + +const canSymlink = symlinksSupported(); + let root: string; let warnings: string[]; const logger = { warn: (message: string) => warnings.push(message) }; @@ -260,6 +276,27 @@ describe("resolveConfigFile", () => { expect(warning.slice(0, warning.indexOf("are shadowed by"))).not.toContain("streamctl.config.js"); }); + it.skipIf(!canSymlink)("stays descriptive when the two spellings are one file", async () => { + // `statSync` follows symlinks, so both candidates pass `isFile` and the warning + // names a single file as shadowing itself under two paths. Cosmetic — but only + // because the message describes rather than instructs. The `shadowedBy` precedent + // this warning is modelled on says "port and delete ", which here would + // tell someone to delete the file their config actually lives in, leaving a + // dangling symlink. Nothing else pins that distinction, so this does. + await writeRootConfig(".ts"); + await symlink(join(root, "streamctl.config.ts"), join(root, "streamctl.config.js")); + + await resolveConfigFile(root, logger); + + const [warning] = warnings; + expect(warning).toContain("is the one being read"); + // The property, not a fixed string: asserting the exact sentence would red on any + // rewording and train the next person to update the expectation rather than think. + for (const verb of ["delete", "remove", "port", "drop", "move", "rename", "fix", "run"]) { + expect(warning.toLowerCase(), verb).not.toMatch(new RegExp(`\\b${verb}\\b`, "u")); + } + }); + it("says nothing when one extension exists", async () => { await writeRootConfig(".ts"); From 4a26103365e9fa42e241a822f9dbd6963024b4e9 Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Thu, 30 Jul 2026 20:08:01 +0200 Subject: [PATCH 28/28] docs: correct what the tripwire comment claims about relocation The comment said a faithful fixture relocation goes 57/57 green the moment the root-fixture assertion is removed. Re-measured in an isolated worktree and it does not reproduce: 4 red with the assertion present, 4 red with it deleted, 3 red with it deleted and `force` added to the `rm`. The protection is defence in depth, not one assertion. The `rm` without `force` raises ENOENT on its own once the default has moved, with no assertion involved -- so it is a second independent guard, and adding `force` is the obvious "fix" this comment exists to argue against. Wrong in the safe direction: it understated the protection. Corrected rather than deleted, because a comment asserting a measurement nobody can repeat is worse than no comment at all. --- test/upgrade.test.ts | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/test/upgrade.test.ts b/test/upgrade.test.ts index de08784..49472de 100644 --- a/test/upgrade.test.ts +++ b/test/upgrade.test.ts @@ -997,17 +997,23 @@ describe("runUpgrade: legacy config location", () => { // (see the comment on `writeConfig`); the root-repo tests above are what can, and // only while the default fixture stays at the root. // - // Do not expect to notice if you move it. Measured: a *faithful* relocation — this - // helper, `readConfig`, `hashSnapshot`, the `NOT_INITIALIZED` teardown, the `pkgDir` - // blocks and the three path-detail expectations, all handled properly — is 57/57 - // green on the first run with this assertion removed, and a full `P02-T02` revert on - // top of that is *also* 57/57 green. There is no red stage to catch it at: the path - // details are part of doing the relocation correctly, not a warning that something - // is wrong. This assertion is the only thing between a competent fixture move and - // total loss of that guard, which is why it earns its place despite proving nothing - // about the product. + // Moving it is defended in depth, and this assertion is only the first layer. + // Measured against a *faithful* relocation — this helper, `readConfig`, + // `hashSnapshot` and every path-detail expectation, all handled properly: // - // No `force` on the `rm` either: without it, a moved default no-ops silently here. + // relocation, assertion present -> 4 red (this block) + // assertion deleted -> still 4 red + // assertion deleted + `force` on the `rm` -> still 3 red + // + // So the `rm` below is a second, independent guard: with the default moved, it + // raises ENOENT on its own, with no assertion involved. That is why it has no + // `force` — adding one is the obvious way to "fix" the resulting failure, and it + // is the step this comment exists to argue against. + // + // An earlier version of this comment claimed the relocation goes 57/57 green the + // moment this assertion is removed. That does not reproduce; it understates the + // protection rather than overstating it. Corrected rather than deleted, because a + // comment asserting a measurement nobody can repeat is worse than no comment. expect(existsSync(join(repo, `${CONFIG_FILE}.ts`)), "the default fixture must stay at the root").toBe(true); await rm(join(repo, `${CONFIG_FILE}.ts`)); await mkdir(join(repo, ".streamctl"), { recursive: true });