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..815efd4 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,30 @@ 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`, …). + +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 +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 Add the gate to your pipeline; exit `3` means the tree drifted from the payload: @@ -232,10 +256,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..16c35d3 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,40 @@ 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/` 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 + 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 moves the `--json` envelope, exit codes, or the manifest `schemaVersion`) so the history reads clearly for consumers. 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 diff --git a/scripts/e2e-dry-run.mjs b/scripts/e2e-dry-run.mjs index 4988119..efe113c 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,16 @@ // 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 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"; import { createHash } from "node:crypto"; import { cpSync, existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; @@ -91,13 +98,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 +124,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 +273,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/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/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/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/resolve.ts b/src/config/resolve.ts new file mode 100644 index 0000000..bd1767d --- /dev/null +++ b/src/config/resolve.ts @@ -0,0 +1,149 @@ +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; + } +} + +/** + * 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)) { + matches.push(abs); + } + } + return matches; +} + +/** + * 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 rootMatches = probeAll(cwd, CONFIG_FILE); + // Probed even on a root hit: it is the only signal for the both-present warning. + 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("/"); + + // 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`, so there is no "skip the resolved file" check + // here: it could never fire, and a guard that cannot fire reads as protection that is + // not there. + // + // Two separate properties hold this up, and they cover different hazards. + // + // 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.`, + ); + } + + 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 === undefined ? "legacy" : "root" }; +} 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/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/src/engine/init.ts b/src/engine/init.ts index aeba3af..74465b4 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 { CONFIG_FILE, 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,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. */ @@ -317,7 +321,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 +334,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 +451,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/src/engine/upgrade.ts b/src/engine/upgrade.ts index 4625589..5b8a98e 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,10 +318,11 @@ 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 - * 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. + * 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. * * 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. @@ -331,7 +332,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); + // `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 @@ -426,9 +429,11 @@ export async function runUpgrade(opts: RunUpgradeOptions): Promise 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..978db07 --- /dev/null +++ b/test/ambiguity.command.test.ts @@ -0,0 +1,137 @@ +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. + 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"); + 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); + }); +}); + +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 }; + 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:"); + + 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/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/helpers/streams.ts b/test/helpers/streams.ts new file mode 100644 index 0000000..ac5b6f5 --- /dev/null +++ b/test/helpers/streams.ts @@ -0,0 +1,42 @@ +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. + * + * **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 + * 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[] = []; + 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"); +} diff --git a/test/init.test.ts b/test/init.test.ts index c976093..3cb2ef3 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"; @@ -7,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. @@ -104,12 +106,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 +148,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 +210,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 () => { @@ -212,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 24 stats every time, 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); @@ -219,7 +279,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 +291,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 +340,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 +377,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 +446,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 +465,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 +500,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 +536,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")); diff --git a/test/load.test.ts b/test/load.test.ts index 91bacb6..fc882fc 100644 --- a/test/load.test.ts +++ b/test/load.test.ts @@ -1,24 +1,81 @@ +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"; 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"; +import { sideEffectConfig, VALID_BODY } from "./helpers/configs"; 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"); 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); 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 +88,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 +98,144 @@ describe("loadStreamctlConfig", () => { expect((error as StreamctlError).code).toBe("CONFIG_INVALID"); expect((error as StreamctlError).message).toContain("versionSyncExclude"); }); + + describe("location", () => { + 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")); + }); + + 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", + // 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, + 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("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. 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); + }); + + 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"); + }); + }); }); diff --git a/test/manifest-schema.test.ts b/test/manifest-schema.test.ts index 13575c7..5b7dd35 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, @@ -21,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); } @@ -101,6 +107,87 @@ describe("ManagedFile", () => { expect(issuePaths({ ...managedFile(), path: "package.json" }, managedFileSchema)).toContain("path"); }); + describe("reserved config paths", () => { + 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", () => { + 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"); + } + }); + + // 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", "ESLint.config.ts"]) { + expect(accepts(path), path).toBe(true); + } + }); + + // 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", () => { + // `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", () => { + 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"); }); diff --git a/test/resolve.test.ts b/test/resolve.test.ts new file mode 100644 index 0000000..efafd54 --- /dev/null +++ b/test/resolve.test.ts @@ -0,0 +1,513 @@ +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"; +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"; + +/** + * 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(); + +/** 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) }; + +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 () => { + // 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 }); +}); + +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); +} + +/** 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); +} + +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 () => { + // 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 () => { + // 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"); + // 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 () => { + 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); + }); + + 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. + // 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); + } + }); + }); + + 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.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"); + + 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", () => { + // 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([]); + }); + }); + + /** + * 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/`. + * + * 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 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 () => { + // 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. + // + // 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([]); + }); + + 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). + // + // 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(); + 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, 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"); + + 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 + // 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 + }); + }); +}); 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 021c2dd..49472de 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 { @@ -63,7 +74,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 +94,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 +105,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 +236,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 +298,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 +306,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 +327,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 +359,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 +469,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); }); @@ -714,6 +724,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); @@ -946,7 +962,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 +976,113 @@ 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 { + // 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. + // + // 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: + // + // 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 }); + 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("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 () => { + 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); + }); +});