Skip to content

Commit daa25b8

Browse files
committed
fix(cli): close the review findings on the release-train coupling
Seven findings from the code-review pass, all addressed: 1. compareVersions gets a strict shape gate (^\d+\.\d+\.\d+ with optional well-formed prerelease; build metadata rejected): empty prerelease ('1.0.0-'), '+metadata', truncated/extra cores, and parseInt trailing garbage ('9.9.10rc') all previously slipped the NaN-only guard and could classify a malformed install as 'ahead', suppressing the reinstall guidance. One gate now encodes the whole not-comparable policy; the NaN check is subsumed. Test matrix covers every class. 2. Mixed ahead+missing no longer silently manufactures a cross-train mismatch: when missing packages are about to be installed beside newer ones, the warning says the pairing may not match and to update stash first. 3. The ahead warning prints the exact update command (devInstallCommand of stash@<highest installed>) — lockstep guarantees that release exists — instead of an uncommanded "matching release". 4. @cipherstash/prisma-next JOINS the fixed group (per Dan: the prisma-next package should line up to the train version too). The changeset's "can never go stale" claim is now actually true — no residual exclusion. 5. release-train.test.ts gains the growth guard: the changesets fixed group must equal RELEASE_TRAIN_MANIFESTS exactly, so a future train package can't silently version outside the lockstep. 6. skills/stash-cli skew sentence is direction-aware (align when older; update stash — never downgrade — when newer), fixing the now-wrong unconditional "align versions before continuing". 7. Companion changesets for @cipherstash/wizard and @cipherstash/prisma-next so their CHANGELOGs explain the version-line jump onto the train. Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
1 parent 83ec7ea commit daa25b8

10 files changed

Lines changed: 141 additions & 27 deletions

File tree

.changeset/config.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
"@cipherstash/stack",
99
"@cipherstash/stack-drizzle",
1010
"@cipherstash/stack-supabase",
11+
"@cipherstash/prisma-next",
1112
"@cipherstash/wizard"
1213
]
1314
],
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
'@cipherstash/prisma-next': patch
3+
---
4+
5+
`@cipherstash/prisma-next` now versions in lockstep with the Stack release
6+
train (`stash`, `@cipherstash/stack`, and the other adapters) via a Changesets
7+
`fixed` group — `stash init` installs it pinned by exact version, so the two
8+
must always release together. This moves the package from its previous `0.4.x`
9+
line onto the shared train version; no API changes.

.changeset/release-train-coupling.md

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,19 @@ Two guards for the release-train version embed (#661 follow-up):
77
**Direction-aware version skew.** `stash init` now distinguishes an installed
88
package that is *behind* this CLI release (offered alignment / the pinned
99
install command, as before) from one that is *newer* than the release expects.
10-
A newer install no longer produces a downgrade command — init says the install
11-
is likely fine and to update the stash CLI to the matching release instead.
12-
Unreadable manifests still count as behind (a broken install should be offered
13-
the reinstall fix).
10+
A newer install no longer produces a downgrade command — init prints the exact
11+
`stash` update command instead (release-train lockstep guarantees that version
12+
exists), and when missing packages are about to be installed alongside newer
13+
ones it says the pairing may not match and to update `stash` first. Unreadable
14+
or malformed manifest versions always count as behind (a broken install should
15+
be offered the reinstall fix, never "looks newer, leave it").
1416

1517
**Version lockstep.** The release-train packages (`stash`,
1618
`@cipherstash/stack`, `@cipherstash/stack-drizzle`,
17-
`@cipherstash/stack-supabase`, `@cipherstash/wizard`) are now a Changesets
18-
`fixed` group: a release of any of them republishes all of them at the same
19-
version, so the CLI's embedded version map can never go stale against the
20-
packages it pins (previously a stack-only release would have left the
21-
published CLI embedding — and recommending — outdated versions).
22-
`@cipherstash/prisma-next` stays on its own version line by design; the
23-
direction-aware messaging above covers it.
19+
`@cipherstash/stack-supabase`, `@cipherstash/prisma-next`,
20+
`@cipherstash/wizard`) are now a Changesets `fixed` group: a release of any of
21+
them republishes all of them at the same version, so the CLI's embedded
22+
version map can never go stale against the packages it pins (previously a
23+
runtime-package-only release would have left the published CLI embedding —
24+
and recommending — outdated versions). A test now asserts the fixed group
25+
stays exactly equal to the release train.
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
'@cipherstash/wizard': patch
3+
---
4+
5+
`@cipherstash/wizard` now versions in lockstep with the Stack release train
6+
(`stash`, `@cipherstash/stack`, and the adapters) via a Changesets `fixed`
7+
group — the `stash` CLI executes the wizard by exact version, so the two must
8+
always release together. This moves the package from its previous `0.5.x`
9+
line onto the shared train version; no API changes.

packages/cli/src/__tests__/release-train.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { INTEGRATION_ADAPTER_PACKAGES } from '../commands/init/steps/install-dep
66
import { RELEASE_TRAIN_MANIFESTS } from '../release-train.js'
77

88
const CLI_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '../..')
9+
const REPO_ROOT = resolve(CLI_ROOT, '../..')
910

1011
// The growth guard for #661: `RELEASE_TRAIN_MANIFESTS` is the single source
1112
// the build embeds versions from, and `INTEGRATION_ADAPTER_PACKAGES` is what
@@ -46,4 +47,20 @@ describe('release train coverage', () => {
4647
expect((m.version as string).length).toBeGreaterThan(0)
4748
}
4849
})
50+
51+
it('the changesets fixed group is exactly the release train (staleness guard)', () => {
52+
// The staleness half of #661/#669 is release-process config: every train
53+
// package must version in lockstep with `stash`, or a release of the odd
54+
// one out ships while the published CLI still embeds — and pins — its old
55+
// version. Guard the config like the code: group membership must equal
56+
// the train, exactly.
57+
const config = JSON.parse(
58+
readFileSync(resolve(REPO_ROOT, '.changeset/config.json'), 'utf8'),
59+
) as { fixed?: string[][] }
60+
const group = config.fixed?.find((g) => g.includes('stash'))
61+
expect(group, 'a changesets fixed group containing stash').toBeDefined()
62+
expect(new Set(group)).toEqual(
63+
new Set(Object.keys(RELEASE_TRAIN_MANIFESTS)),
64+
)
65+
})
4966
})

packages/cli/src/__tests__/runtime-versions.test.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,12 +116,26 @@ describe('compareVersions', () => {
116116
expect(compareVersions('1.0.0-rc.1', '1.0.0-rc.1')).toBe(0)
117117
})
118118

119-
it('treats a non-numeric core as not comparable (never "ahead")', () => {
120-
// A corrupt manifest version must not NaN-poison the order into 1
121-
// ("installed is newer") — that would suppress the align guidance.
119+
it('treats any malformed version as not comparable (never "ahead")', () => {
120+
// A corrupt manifest version must not partially parse into 1 ("installed
121+
// is newer") — that would suppress the align guidance. The strict shape
122+
// gate rejects every malformed class, not just NaN-able cores:
122123
expect(compareVersions('v1.0.0', '1.0.0')).toBe(0)
123124
expect(compareVersions('1.0.x', '1.0.0')).toBe(0)
124125
expect(compareVersions('garbage', '1.0.0')).toBe(0)
126+
// truncated core — '1.0' padded to 1.0.0 would outrank 1.0.0-rc.2
127+
expect(compareVersions('1.0', '1.0.0-rc.2')).toBe(0)
128+
// extra core segment
129+
expect(compareVersions('1.2.3.4', '1.2.3')).toBe(0)
130+
// empty prerelease — '1.0.0-' as a release would outrank any rc
131+
expect(compareVersions('1.0.0-', '1.0.0-rc.1')).toBe(0)
132+
// parseInt trailing garbage — '9.9.10rc' would parse as core 9.9.10
133+
expect(compareVersions('9.9.10rc', '9.9.9')).toBe(0)
134+
expect(compareVersions('1.0.0beta', '1.0.0-rc.1')).toBe(0)
135+
// build metadata — '2+sha' as an identifier would mis-order vs 'rc.10'
136+
expect(compareVersions('1.0.0-rc.2+sha.abc', '1.0.0-rc.10')).toBe(0)
137+
// empty prerelease identifier
138+
expect(compareVersions('1.0.0-rc..1', '1.0.0-rc.1')).toBe(0)
125139
})
126140

127141
it('numeric identifiers sort below alphanumeric; shorter below longer', () => {

packages/cli/src/commands/init/steps/__tests__/install-deps.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ vi.mock('../../utils.js', () => ({
1313
...(dev.length ? [`npm install --save-dev ${dev.join(' ')}`] : []),
1414
],
1515
),
16+
devInstallCommand: vi.fn(
17+
(_pm: string, pkg: string) => `npm install -D ${pkg}`,
18+
),
1619
detectPackageManager: vi.fn(() => 'npm'),
1720
}))
1821
// Pin map: pretend this CLI release was built alongside these versions, so
@@ -234,12 +237,43 @@ describe('installDepsStep', () => {
234237
'@cipherstash/stack: installed 9.9.10 is newer than this release of stash expects (9.9.9-test.1)',
235238
),
236239
)
240+
// Lockstep means stash@<installed> must exist — the warning prints the
241+
// exact update command instead of an uncommanded "matching release".
242+
expect(p.log.warn).toHaveBeenCalledWith(
243+
expect.stringContaining('npm install -D stash@9.9.10'),
244+
)
237245
// No align/downgrade guidance, no mutation, clean success.
238246
expect(p.note).not.toHaveBeenCalled()
239247
expect(execSyncMock).not.toHaveBeenCalled()
240248
expect(result.stackInstalled).toBe(true)
241249
})
242250

251+
it('mixed ahead + missing: warns that fresh installs pin to THIS release and may mismatch', async () => {
252+
vi.mocked(isInteractive).mockReturnValue(false)
253+
// stack is installed and NEWER; the supabase adapter is missing — init
254+
// will install the adapter at this CLI's older embed, so it must say the
255+
// pairing may not match rather than silently manufacturing a cross-train
256+
// mismatch under a "likely fine" banner.
257+
present('@cipherstash/stack', 'stash')
258+
resolvedVersions({
259+
'@cipherstash/stack': '9.9.10',
260+
stash: FIXTURE_VERSIONS.stash,
261+
})
262+
263+
await installDepsStep.run(baseState, supabaseProvider)
264+
265+
expect(p.log.warn).toHaveBeenCalledWith(
266+
expect.stringContaining(
267+
"@cipherstash/stack-supabase will be installed at THIS release's versions",
268+
),
269+
)
270+
expect(p.log.warn).toHaveBeenCalledWith(
271+
expect.stringContaining('npm install -D stash@9.9.10'),
272+
)
273+
// The missing adapter still installs (non-interactive default).
274+
expect(execSyncMock).toHaveBeenCalled()
275+
})
276+
243277
it('reports an unreadable manifest as skew, not as a matching install', async () => {
244278
vi.mocked(isInteractive).mockReturnValue(false)
245279
present('@cipherstash/stack', 'stash')

packages/cli/src/commands/init/steps/install-deps.ts

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { CancelledError } from '../types.js'
1212
import {
1313
combinedInstallCommands,
1414
detectPackageManager,
15+
devInstallCommand,
1516
installedVersion,
1617
isPackageInstalled,
1718
} from '../utils.js'
@@ -191,11 +192,6 @@ export const installDepsStep: InitStep = {
191192
if (skewed.length > 0) {
192193
p.log.warn(`Version skew detected:\n ${skewLines(skewed)}`)
193194
}
194-
if (ahead.length > 0) {
195-
p.log.warn(
196-
`Installed versions are newer than this release of stash:\n ${aheadLines(ahead)}\nYour installs are likely fine — update the stash CLI to the matching release instead of downgrading.`,
197-
)
198-
}
199195

200196
// What's missing outright (pinned, prod/dev split).
201197
const missing: string[] = []
@@ -204,6 +200,29 @@ export const installDepsStep: InitStep = {
204200
if (!cliPresent) missing.push(CLI_PACKAGE)
205201
const missingSplit = splitProdDev(missing)
206202

203+
if (ahead.length > 0) {
204+
// Every release-train package versions in lockstep (the changesets
205+
// `fixed` group), so a train package strictly ahead of this CLI's embed
206+
// implies a stash release exists at that exact version — print the
207+
// command instead of leaving the user to research "the matching
208+
// release". Highest ahead version wins when several differ.
209+
const target = ahead
210+
.map(({ installed }) => installed)
211+
.reduce((max, v) => (compareVersions(v, max) > 0 ? v : max))
212+
const updateCmd = devInstallCommand(pm, `stash@${target}`)
213+
// Installing MISSING packages now would pin them to this CLI's older
214+
// embed, pairing them with the newer installed packages — a combination
215+
// no lockstep release ever shipped. Say so instead of silently
216+
// manufacturing the mismatch.
217+
const missingNote =
218+
missing.length > 0
219+
? `\nNote: ${missing.join(', ')} will be installed at THIS release's versions, which may not match the newer packages above — for a consistent set, update stash first and re-run init:\n ${updateCmd}`
220+
: `\nUpdate with:\n ${updateCmd}\nthen re-run init.`
221+
p.log.warn(
222+
`Installed versions are newer than this release of stash:\n ${aheadLines(ahead)}\nYour installs are likely fine — update the stash CLI to the matching release instead of downgrading.${missingNote}`,
223+
)
224+
}
225+
207226
// Interactively, skewed packages can be aligned in the same install run.
208227
// Non-interactive runs never mutate an existing install: agents/CI get
209228
// the warning + exact commands above and keep going.

packages/cli/src/runtime-versions.ts

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -104,13 +104,22 @@ export function pinnedSpec(
104104
* (`x.y.z` and `x.y.z-rc.n`), so the skew warning can tell "behind" from
105105
* "ahead" without adding a dependency to the CLI.
106106
*
107-
* A version whose core isn't numeric (`v1.0.0`, `1.0.x`, garbage from a
108-
* corrupt manifest) is NOT COMPARABLE: return `0` rather than a
109-
* NaN-poisoned order. Callers classify non-ahead as behind, so an
110-
* unparseable installed version gets the safe treatment (align/reinstall
107+
* A version that isn't strictly `digits.digits.digits` with an optional
108+
* well-formed prerelease (`v1.0.0`, `1.0.x`, `1.0`, `1.0.0-`, `1.0.0beta`,
109+
* `1.0.0-rc.2+sha` — build metadata deliberately rejected, and any other
110+
* garbage from a corrupt manifest) is NOT COMPARABLE: return `0` rather
111+
* than a partially-parsed order. Callers classify non-ahead as behind, so
112+
* an unparseable installed version gets the safe treatment (align/reinstall
111113
* guidance) instead of being silently promoted to "newer, leave it".
112114
*/
115+
/** Exactly three numeric core segments, optional dot-separated prerelease
116+
* identifiers (each non-empty alphanumeric/hyphen). No build metadata: `+`
117+
* fails the shape and the version is treated as not comparable — the safe
118+
* direction — rather than mis-ordered by an identifier like `2+sha`. */
119+
const VERSION_SHAPE = /^\d+\.\d+\.\d+(-[0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?$/
120+
113121
export function compareVersions(a: string, b: string): -1 | 0 | 1 {
122+
if (!VERSION_SHAPE.test(a) || !VERSION_SHAPE.test(b)) return 0
114123
const parse = (v: string) => {
115124
const [core, ...pre] = v.split('-')
116125
return {
@@ -120,10 +129,10 @@ export function compareVersions(a: string, b: string): -1 | 0 | 1 {
120129
}
121130
const pa = parse(a)
122131
const pb = parse(b)
123-
if (pa.core.some(Number.isNaN) || pb.core.some(Number.isNaN)) return 0
124132
for (let i = 0; i < 3; i++) {
125-
const da = pa.core[i] ?? 0
126-
const db = pb.core[i] ?? 0
133+
// The shape gate guarantees exactly three numeric segments.
134+
const da = pa.core[i] as number
135+
const db = pb.core[i] as number
127136
if (da !== db) return da < db ? -1 : 1
128137
}
129138
// Same core: a release (no prerelease) outranks any prerelease of it.

skills/stash-cli/SKILL.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ npx stash init # PostgreSQL / Drizzle / Prisma
3636
npx stash init --supabase # Supabase
3737
```
3838

39-
`stash init` installs the CLI as a project dev dependency, so subsequent commands can drop the `npx`. The CLI is package-manager aware — before init, use whichever one-shot runner your project uses (`npx`, `pnpm dlx`, `bunx`, `yarn dlx`). Installs are **pinned to the exact `@cipherstash/*` versions this CLI release shipped with** (never bare dist-tags, which can lag behind a release), and init warns if an already-installed package's resolved version differs from the release's — treat that warning as a real problem and align versions before continuing.
39+
`stash init` installs the CLI as a project dev dependency, so subsequent commands can drop the `npx`. The CLI is package-manager aware — before init, use whichever one-shot runner your project uses (`npx`, `pnpm dlx`, `bunx`, `yarn dlx`). Installs are **pinned to the exact `@cipherstash/*` versions this CLI release shipped with** (never bare dist-tags, which can lag behind a release), and init warns if an already-installed package's resolved version differs from the release's. Treat that warning as a real problem — but the fix depends on direction, and init says which applies: an **older** install should be aligned to the release (init offers the exact command); a **newer** install must NOT be downgraded — update the `stash` CLI to the matching release instead (init prints that command too).
4040

4141
**If you are an agent, do this first:**
4242

0 commit comments

Comments
 (0)