feat(config): default to streamctl.config.ts in the repo root - #7
Merged
Conversation
Probe streamctl.config.<ext> at the invocation directory, then .streamctl/config.<ext>, over c12's own SUPPORTED_EXTENSIONS. Root wins when both exist and one warning names both paths. Probing before any load is what keeps c12's .config/ fallbacks unreachable.
Nothing under .config/ resolves, and a repo holding both a .config/ file and a legacy config still reads the legacy one. The exclusion is structural, so these characterization tests are its only guard.
c12 introduced _configFile in 3.2.0, and load.ts uses it as the existence signal. Below that floor the field is always undefined, so every command threw NOT_INITIALIZED for a config that loads fine — hitting any consumer that resolved c12 via @prisma/config's 3.1.0 pin.
loadStreamctlConfig resolves the location first, calls c12 exactly once
for the confirmed spelling, and returns { config, location } so writers
can act on the file that was actually read. An uninitialized repo no
longer reaches the loader at all. CONFIG_INVALID copy is now
location-neutral.
The probe and c12 must agree on which file won; they are compared as
realpath'd, POSIX-normalized paths, since c12 emits pathe-normalized
paths and exsolve resolves symlinks.
A regression that loaded speculatively and rejected .config/ hits afterwards would still return null while having executed the file. The sentinel body catches it. Also records where the /index divergence is decided, so deleting that test reads as a spec change.
The four loader call sites take { config, location } and pass their
reporter as the logger, so the both-present warning reaches stderr and
stays out of the --json envelope on stdout. NOT_INITIALIZED now names
both accepted locations.
The rollback snapshot and the version pin bump both take the resolved location, so a legacy repo's transaction covers .streamctl/config.ts instead of a root path that never existed — which restoreFile would have reported as a successful no-op restore.
…ement A payload managing the config would have sync overwrite the file the same run just read, and fight upgrade's rollback snapshot. Reservations compare a posix-normalized path, so ./ and .// spellings of the same file cannot slip past — which also closes that hole in the existing package.json reservation.
init writes the config at the invocation directory and no longer creates a .streamctl/ directory. The already-initialized guard consults the resolver, so a config in either location blocks a second init and the error names the file that is actually there.
Also pins details.path on the ambiguous-pin branch and corrects the posix.normalize rationale: on Windows the platform normalize would turn the legacy prefix into a backslash path, silently disabling that reservation rather than tripping the backslash check.
A case variant such as PACKAGE.JSON or .STREAMCTL/config.ts named the reserved file on macOS and Windows while passing the guard, letting a payload manage the config that sync then overwrites. Lowercasing inside the shared normalization covers all three reservations at once. Also label the reservation assertions with the path under test: the failure fires inside the issuePaths helper, so a call-site label never renders.
A literal spelling in the write path let init scaffold a file the resolver no longer looks for. Also assert stderr in the resolver's no-logger test, which watched only the injected array and stayed green under the house-default fallback it exists to forbid.
Mechanical, safe to scan: four config fixtures relocated with git mv (renames, zero-line diffs), the two command temp-repo builders repointed, and load.test.ts's location tests split into a root case and a legacy one against the new fixture. Four additions worth reading closely: - a loader-level eval-once test. resolve.test.ts asserts the same property, but the resolver only stats and cannot violate it; loadConfig can. Under a two-call loader this is the only failure in 711 tests. - legacy-vs-root parity: the same command against one repo converted in place, asserting identical report output and empty stderr. - sideEffectConfig promoted to test/helpers, with a VALID_BODY constant because its default export fails validation one layer up. - a tripwire keeping upgrade's default fixture at the root, which is the only thing that can detect a legacy-hardcoded pin bump.
Moves the init leg and the scaffolded-repo builder onto streamctl.config.ts and adds an assertion that no .streamctl/ directory is created, which only this gate can check against the built artifact. Leg 4 deliberately stays on the legacy location and now says why: it is the only artifact-level coverage of the fallback on any package manager. Also fixes the header's leg inventory, which claimed three legs where the script runs six, and records in test/load.test.ts why the eval-once test's positive assertion is free only while the loader must evaluate one file.
Sweeps README, docs/ and the two .github/ prompts onto the root default, adds a "Where the config lives" section stating that exactly two locations resolve and that the legacy one is permanent, and drops the .streamctl/** entry from the ESLint ignore example per Q4. The three remaining source comments follow; the six deliberate references in src/ are left alone. Also refreshes docs/release.md's status banner (0.1.0 is on the registry) with notes for the next release, and clarifies in the E2E gate's header that legs 4-6 run once per invocation rather than once per matrix. BREAKING CHANGE: a config at .config/.streamctl/config.ts no longer resolves and now raises NOT_INITIALIZED. Measured against c12 3.3.4: the old `configFile: ".streamctl/config"` spelling made c12 probe `.config/.streamctl/config`, and the new spelling does not. Fix with `git mv .config/.streamctl/config.ts streamctl.config.ts`. Nothing else under .config/ resolved before this release, and nothing does now.
The block's claims about what c12 would do under each spelling were reasoned about for two gates. The table replaces the argument with the measurement, including the counterfactual: under the new spelling raw c12 does reach two of the three .config/ shapes, so the existence probe is the only reason it never gets the chance. Also notes in the release runbook that the broken shape only ever worked on c12 >= 3.2.0, without conditioning the migration step on it.
c12 reads the first supported extension in its own order, so streamctl.config.js silently beats streamctl.config.ts -- the reverse of what most people expect. The probe now collects every match at a location instead of stopping at the first, and names the shadowed siblings alongside the file actually read. Applies at the legacy location too; the losing location is ignored wholesale, so a collision inside it is not reported. The winner cannot appear in its own shadow list: it is matches[0] and the candidates are one spelling crossed with twelve distinct extensions. No "skip the resolved file" filter, because it could never fire and would read as protection that is not there -- the precondition it would appear to guard is candidate distinctness, which is now pinned by its own test. Cost note: the probe no longer short-circuits, so a run makes 24 stats rather than as few as 2. Updated in both places that stated the old number. Also corrects "24 existsSync calls" to statSync().isFile() in 70_risks.md and 90_questions.md. That is the fourth and fifth in the same family -- 20_architecture.md, 40_data_model.md and P03-T04's AC 50 were the others -- all from specs written before the implementation chose isFile. If a sixth turns up, it is this pattern, not a local typo.
It fires on repos that changed nothing -- a streamctl.config.js sitting beside a streamctl.config.ts since before this release starts warning on every command -- so it belongs in the release notes and in the README section that already sets up the trap by listing the accepted extensions without saying .js beats .ts. Both use the word the warning itself prints, so a user who greps the docs for text from the message finds it. Also pins two acceptance criteria that were true but unasserted: the shadow warning is stderr-only, cloned from the cross-location test rather than inherited from a shared code path, and candidate distinctness now covers both spellings, since the warning fires at either location.
status has no warnings[] -- that is SyncResult's, reaching the envelopes of sync and check -- so probing data.warnings guessed a shape this command does not have and missed any leak landing elsewhere. Asserting the serialized envelope is a strict superset and needs no knowledge of the result shape. It also covers what the prefix check cannot, since collectShadowWarnings writes without the streamctl: prefix.
It promised an intermediate signal that does not exist -- "a handful of path-detail assertions" going red, which a reader would take as a moment they might notice. Measured at the Phase 3 gate: a faithful relocation with the assertion removed is 57/57 green on the first run, and a full P02-T02 revert on top is also 57/57. The path details are part of doing the move correctly, not a warning. A comment that understates the hazard invites the "I would have noticed" reasoning the measurement disproves.
The absent "skip the resolved file" filter was justified by candidate distinctness, which is the weaker of the two reasons the code is correct. Under symlink or hardlink aliasing two distinct spellings are one file, and the winner still never lands in its own shadow list -- because it is matches[0] and excluded by position, not by identity. A maintainer switching to identity- or set-based selection would break that while believing the invariant was preserved, since the distinctness test would still pass. It pins the precondition for message quality, not this property.
statSync follows symlinks, so a streamctl.config.js symlinked to streamctl.config.ts passes isFile twice and the warning names one file as shadowing itself under two paths. Measured, not hypothetical. Positional selection removes self-comparison but not this: an alias can appear in the shadowed list under a different path. What keeps that cosmetic is the wording -- "Y is the one being read" states a fact, where the shadowedBy precedent says "port and delete", which in this state would tell someone to delete the file their config lives in and leave a dangling symlink. Nothing pinned that. The test asserts the message carries no imperative verb, rather than matching a fixed sentence that would red on any rewording. The resolver comment now names both properties instead of implying positional selection covers the aliasing case too.
The comment said a faithful fixture relocation goes 57/57 green the moment the root-fixture assertion is removed. Re-measured in an isolated worktree and it does not reproduce: 4 red with the assertion present, 4 red with it deleted, 3 red with it deleted and `force` added to the `rm`. The protection is defence in depth, not one assertion. The `rm` without `force` raises ENOENT on its own once the default has moved, with no assertion involved -- so it is a second independent guard, and adding `force` is the obvious "fix" this comment exists to argue against. Wrong in the safe direction: it understated the protection. Corrected rather than deleted, because a comment asserting a measurement nobody can repeat is worse than no comment at all.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The default config location moves to
streamctl.config.tsin the repo root. The old.streamctl/config.*location still resolves, permanently, with no deprecation and no warning.initscaffolds the root file, and refuses when a config already exists in either location.upgradesnapshots and bumps whichever config was actually loaded, so a rollback in a legacy repo restores the legacy file rather than a root one that was never there.streamctl.config.jsnext tostreamctl.config.ts. It names the one being read. c12 orders.jsahead of.ts, which catches people out. The warning describes what happened and does not tell anyone to delete a file.One thing breaks.
.config/.streamctl/config.tsused to resolve through c12's.config/directory convention, and now raisesNOT_INITIALIZED. The fix is a move:The c12 floor moves to
>=3.2.0. The loader branches on_configFile, which landed in that version.Why
A dotfile directory holding one config file is an odd shape. A root
streamctl.config.tsis what people already expect from vite, vitest and tailwind, and you can see it when you list the repo.Existing repos keep working with no action.