diff --git a/CHANGELOG.md b/CHANGELOG.md index a821d451..3d8edf93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -571,6 +571,12 @@ npm release are grouped under the in-development version that introduced them. ### Fixed +- **`@stitchapi/openapi` refuses to overwrite a file it did not generate.** + ([#694](https://github.com/rejifald/StitchAPI/issues/694)) A re-run wrote every emitted file + unconditionally at exit `0`, so `--out ` silently discarded + hand-written edits. It now reads back the previous run's `.stitch-gen.json` and refuses to touch + anything that manifest does not claim — naming the files, exiting non-zero — with `--force` to opt + in. That manifest also records the validator tier actually **emitted**, not the one requested. - **`axiosAdapter(axios)` — the adapter's own documented snippet — now typechecks against real axios.** ([#708](https://github.com/rejifald/StitchAPI/issues/708)) `AxiosLikeConfig.responseType` was `string`, which axios types as its narrower `ResponseType` union, so passing `axios` (or diff --git a/packages/openapi/README.md b/packages/openapi/README.md index c7092bfd..b262d79f 100644 --- a/packages/openapi/README.md +++ b/packages/openapi/README.md @@ -33,6 +33,7 @@ npx @stitchapi/openapi ./openapi.json --all --dry-run | `--grep ` | only operations whose path contains this substring | | `--layout dir\|flat` | `dir` = one folder per operation (default); `flat` = one file | | `--validator ` | `types-only` (default). valibot/zod tiers are not implemented yet | +| `--force` | overwrite files it did not generate (it refuses by default) | | `--dry-run` | print the files to stdout instead of writing them | ## What it emits @@ -42,7 +43,9 @@ one stitch per operation typed via `stitch()`, and **atomic** component types placed by fan-in — a schema used by ≥2 operations goes to `_shared/`, one used by a single operation lives **inside that operation's directory** so deleting the operation deletes its private types too. A `.stitch-gen.json` manifest records the -ownership graph. +ownership graph and every path the run wrote — so a re-run replaces what it +generated last time and **refuses to overwrite anything else**, naming the files +and exiting non-zero unless you pass `--force`. ``` src/pet-client/ diff --git a/packages/openapi/src/cli.ts b/packages/openapi/src/cli.ts index 90920dce..028f6195 100644 --- a/packages/openapi/src/cli.ts +++ b/packages/openapi/src/cli.ts @@ -1,17 +1,23 @@ // `stitch-openapi` — eject a SELECTED set of operations from an OpenAPI document into ready-to-own // stitch source (ADR 0013). Thin wrapper over the pure `planGen`: parse argv, read the spec (JSON // natively; YAML via the `yaml` dependency, imported lazily), then write the files (or print them -// on --dry-run). -import { type GenOptions, type OpenApiDoc, planGen } from './gen-openapi'; - -import { mkdir, readFile, writeFile } from 'node:fs/promises'; +// on --dry-run). Writing is guarded: a file the previous run's manifest does not claim belongs to +// the author, and is never overwritten without --force. +import { + type GenOptions, + MANIFEST_FILE, + type OpenApiDoc, + planGen, +} from './gen-openapi'; + +import { access, mkdir, readFile, writeFile } from 'node:fs/promises'; import { dirname, resolve } from 'node:path'; const USAGE = `stitch-openapi — eject selected operations from an OpenAPI document into stitch source usage: stitch-openapi --out [--all | --tag | --only | --grep ] - [--layout dir|flat] [--validator types-only] [--dry-run] + [--layout dir|flat] [--validator types-only] [--force] [--dry-run] an OpenAPI 3.x document (JSON or YAML) --out, -o output directory (required unless --dry-run) @@ -21,6 +27,7 @@ usage: --grep only operations whose path contains this substring --layout dir|flat dir = one folder per operation (default); flat = one file per operation --validator types-only (default; v1). valibot/zod tiers are not implemented yet + --force overwrite files this ejector did not generate (it refuses by default) --dry-run print the files to stdout instead of writing them Ejects ready-to-own source you edit afterward: a client.ts seam, one stitch per operation typed @@ -36,6 +43,7 @@ interface Io { writeErr: (s: string) => void; readFileText: (path: string) => Promise; writeFile: (path: string, contents: string) => Promise; + exists: (path: string) => Promise; } function defaultIo(): Io { @@ -48,9 +56,68 @@ function defaultIo(): Io { await mkdir(dirname(path), { recursive: true }); await writeFile(path, contents, 'utf8'); }, + exists: async (path) => { + try { + await access(path); + return true; + } catch { + return false; + } + }, }; } +function asArray(v: unknown): unknown[] { + return Array.isArray(v) ? (v as unknown[]) : []; +} + +/** + * Paths (relative to --out) that a PREVIOUS run of this ejector wrote there, per the manifest it + * left behind. Everything else on disk belongs to the author — an edited `client.ts`, a hand-written + * helper — and is not ours to replace (#694 §1). No manifest means no previous run: own nothing. + */ +async function ownedPaths(io: Io, outDir: string): Promise> { + const owned = new Set(); + let raw: string; + try { + raw = await io.readFileText(resolve(outDir, MANIFEST_FILE)); + } catch { + return owned; + } + let parsed: unknown; + try { + parsed = JSON.parse(raw) as unknown; + } catch { + io.writeErr( + `warning: ${MANIFEST_FILE} is not valid JSON; treating every existing file as yours\n`, + ); + return owned; + } + const m = ( + typeof parsed === 'object' && parsed !== null ? parsed : {} + ) as Record; + + // The exact list, written by every run since the guard shipped. + if (Array.isArray(m['files'])) { + for (const p of asArray(m['files'])) + if (typeof p === 'string') owned.add(p); + return owned; + } + // Older manifests predate that list but still record the ownership graph, so derive from it — + // adopting a tree this ejector really did generate should not cost a blanket --force. The three + // constants are emitted unconditionally by every run (client.ts, index.ts, the manifest itself). + owned.add(MANIFEST_FILE); + owned.add('client.ts'); + owned.add('index.ts'); + for (const e of [...asArray(m['operations']), ...asArray(m['schemas'])]) { + const f = (e as { file?: unknown }).file; + // A flat-layout private schema records `.ts (inlined)` — a marker, not a path. + if (typeof f === 'string' && f !== '' && !f.endsWith('(inlined)')) + owned.add(f); + } + return owned; +} + export async function main( argv: string[], overrides: Partial = {}, @@ -64,6 +131,7 @@ export async function main( let spec: string | undefined; let out: string | undefined; let dryRun = false; + let force = false; const opts: GenOptions = {}; const tags: string[] = []; const only: string[] = []; @@ -87,7 +155,8 @@ export async function main( } else if (a === '--layout') { const v = argv[++i]; if (v) opts.layout = v as NonNullable; - } else if (a === '--dry-run') dryRun = true; + } else if (a === '--force') force = true; + else if (a === '--dry-run') dryRun = true; else if (!a.startsWith('-') && spec === undefined) spec = a; else io.writeErr(`warning: ignored unknown arg: ${a}\n`); } @@ -143,8 +212,32 @@ export async function main( } const base = out as string; + const outDir = resolve(io.cwd, base); + + // Eject, not managed regeneration (ADR 0013): the tree is the author's once written, so a re-run + // may replace only what a previous run put there. Anything else that already exists is theirs. + if (!force) { + const owned = await ownedPaths(io, outDir); + const clobbered: string[] = []; + for (const f of result.files) { + if (owned.has(f.path)) continue; // ours from last time — the normal regen path + if (await io.exists(resolve(outDir, f.path))) + clobbered.push(f.path); + } + if (clobbered.length > 0) { + io.writeErr( + `refusing to overwrite ${clobbered.length} file(s) in ${base} that this generator did not write (no record of them in ${MANIFEST_FILE}):\n`, + ); + for (const p of clobbered) io.writeErr(` ${p}\n`); + io.writeErr( + 'Nothing was written. Eject into a scratch directory and diff (ADR 0013 Decision 1), or pass --force to overwrite them.\n', + ); + return 1; + } + } + for (const f of result.files) - await io.writeFile(resolve(io.cwd, base, f.path), f.contents); + await io.writeFile(resolve(outDir, f.path), f.contents); io.writeErr(`wrote ${result.files.length} file(s) to ${base}\n`); return 0; } diff --git a/packages/openapi/src/gen-openapi.ts b/packages/openapi/src/gen-openapi.ts index 513eb983..9b01892e 100644 --- a/packages/openapi/src/gen-openapi.ts +++ b/packages/openapi/src/gen-openapi.ts @@ -93,6 +93,9 @@ const HTTP_METHODS = [ 'trace', ] as const; +/** The ownership manifest, written into --out (ADR 0013 Decision 9). Read back by the CLI. */ +export const MANIFEST_FILE = '.stitch-gen.json'; + // ---- options & result ----------------------------------------------------- export interface GenOptions { @@ -408,12 +411,17 @@ function directRefs(schema: SchemaNode | undefined, acc: Set): void { export function planGen(doc: OpenApiDoc, opts: GenOptions = {}): GenResult { const warnings: string[] = []; const notices: string[] = []; - const validator = opts.validator ?? 'types-only'; + const requested = opts.validator ?? 'types-only'; + // The tier actually EMITTED. v1 has no valibot/zod emitter, so it is always types-only — and + // the manifest, which is the durable artefact, records THIS rather than what was asked for. + // Recording the request left `"validator": "zod"` sitting over a tree with no validators in it, + // with only a build-time stderr line to contradict it (#694 §3). + const validator = 'types-only'; const layout = opts.layout ?? 'dir'; - if (validator !== 'types-only') { + if (requested !== validator) { warnings.push( - `validator "${validator}" is not implemented in v1; emitting types-only`, + `validator "${requested}" is not implemented in v1; emitting types-only`, ); } notices.push( @@ -629,6 +637,11 @@ export function planGen(doc: OpenApiDoc, opts: GenOptions = {}): GenResult { generator: 'stitch gen openapi', validator, layout, + // Every path this run writes, relative to --out, INCLUDING this manifest. It is the durable + // record of what the ejector OWNS: the CLI reads it back on the next run and refuses to + // overwrite anything absent from it (#694 §1). The ownership graph below records file names + // too, but only for operations and non-inlined schemas — this list is exact by construction. + files: [...files.map((f) => f.path), MANIFEST_FILE].sort(), operations: selected.map((o) => ({ name: o.name, method: o.method, @@ -643,7 +656,7 @@ export function planGen(doc: OpenApiDoc, opts: GenOptions = {}): GenResult { schemas: schemaManifest, }; files.push({ - path: '.stitch-gen.json', + path: MANIFEST_FILE, contents: `${JSON.stringify(manifest, null, 2)}\n`, }); @@ -966,6 +979,7 @@ function emptyManifest(validator: string, layout: string): unknown { generator: 'stitch gen openapi', validator, layout, + files: [], operations: [], schemas: [], }; diff --git a/packages/openapi/test/cli.spec.ts b/packages/openapi/test/cli.spec.ts new file mode 100644 index 00000000..c276c0be --- /dev/null +++ b/packages/openapi/test/cli.spec.ts @@ -0,0 +1,192 @@ +// The ejector's write guard (#694 §1). Eject is not managed regeneration: once written, the tree +// belongs to the author, so a re-run may replace only what a PREVIOUS run put there — which +// `.stitch-gen.json` records. Anything else that already exists is the author's, and replacing it +// silently is data loss: the reported case lost a hand-added `drift(Order)` output, after which a +// response missing a `required` field started passing. +// +// Driven through `main(argv, io)` with an in-memory IO, in the style of core's `stitch init` specs. +// One test at the bottom runs against a REAL temp directory, so the default IO's existence probe is +// covered too rather than only the fake that stands in for it. +import { main } from '../src/cli'; + +import { + mkdir, + mkdtemp, + readFile, + readdir, + rm, + writeFile, +} from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; + +const SPEC = JSON.stringify({ + openapi: '3.0.0', + info: { title: 'T', version: '1' }, + servers: [{ url: 'https://api.example.com' }], + paths: { + '/orders/{id}': { + get: { operationId: 'getOrder', responses: { '200': {} } }, + }, + }, +}); + +// The same document plus a second operation, so a later run wants a file that is already on disk. +const GROWN = JSON.stringify({ + ...(JSON.parse(SPEC) as Record), + paths: { + '/orders/{id}': { + get: { operationId: 'getOrder', responses: { '200': {} } }, + }, + '/orders': { + get: { operationId: 'listOrders', responses: { '200': {} } }, + }, + }, +}); + +// What an author's own client.ts looks like: edited, and nothing like the emitted one. +const MINE = "// mine\nexport const client = 'hand-written';\n"; + +const SPEC_PATH = 'openapi.json'; +/** An emitted path as the fake filesystem keys it — `main` resolves against cwd + --out. */ +const at = (p: string): string => resolve('/repo', 'out', p); + +// A fake filesystem backing the CLI's IO: writeFile mutates the map, exists/readFileText read it. +// Returns the map (so a tree can be carried into a second run) plus captured stderr. +const runGen = ( + args: string[] = [], + seed: Record = { [SPEC_PATH]: SPEC }, +) => { + const files = new Map(Object.entries(seed)); + const err: string[] = []; + return main([SPEC_PATH, '--all', '--out', 'out', ...args], { + cwd: '/repo', + write: () => undefined, + writeErr: (s) => err.push(s), + readFileText: async (path) => { + const v = files.get(path); + if (v === undefined) throw new Error(`ENOENT: ${path}`); + return v; + }, + writeFile: async (path, contents) => { + files.set(path, contents); + }, + exists: async (path) => files.has(path), + }).then((code) => ({ code, err: err.join(''), files })); +}; + +describe('cli — refusing to clobber a file it does not own (#694 §1)', () => { + test('an existing file no manifest claims blocks the whole write, non-zero', async () => { + const { code, err, files } = await runGen([], { + [SPEC_PATH]: SPEC, + [at('client.ts')]: MINE, + }); + + expect(code).not.toBe(0); + expect(err).toMatch(/refusing to overwrite/); + expect(err).toMatch(/client\.ts/); // names the file it would have destroyed + expect(err).toMatch(/--force/); // and the way out + // Nothing was written at all — not the manifest, not the operation files. + expect(files.get(at('client.ts'))).toBe(MINE); + expect([...files.keys()]).toEqual([SPEC_PATH, at('client.ts')]); + }); + + test('--force overwrites it', async () => { + const { code, files } = await runGen(['--force'], { + [SPEC_PATH]: SPEC, + [at('client.ts')]: MINE, + }); + + expect(code).toBe(0); + expect(files.get(at('client.ts'))).not.toBe(MINE); + expect(files.get(at('client.ts'))).toMatch(/seam\(/); + expect(files.has(at('.stitch-gen.json'))).toBe(true); + }); + + test('a file the manifest owns regenerates without --force', async () => { + const first = await runGen(); + expect(first.code).toBe(0); + const emitted = first.files.get(at('client.ts')) as string; + + // Carry that tree into a second run, with an owner's edit on top. It is still OURS by the + // manifest, so the re-run replaces it — that is eject-and-diff (ADR 0013 Decision 1). + const tree = Object.fromEntries(first.files); + tree[at('client.ts')] = `${emitted}// edited by the owner\n`; + const second = await runGen([], tree); + + expect(second.code).toBe(0); + expect(second.err).not.toMatch(/refusing/); + expect(second.files.get(at('client.ts'))).toBe(emitted); + expect(second.files.get(at('get-order/index.ts'))).toMatch( + /client\.stitch/, + ); + }); + + test('one unowned file blocks the write even when the rest of the tree is owned', async () => { + const first = await runGen(); + // The spec grows an operation, and the author had already hand-written that file. + const tree = Object.fromEntries(first.files); + tree[SPEC_PATH] = GROWN; + tree[at('list-orders/index.ts')] = MINE; + + const { code, err, files } = await runGen([], tree); + + expect(code).not.toBe(0); + expect(err).toMatch(/list-orders\/index\.ts/); + expect(err).not.toMatch(/get-order\/index\.ts/); // that one is ours; only the new file is not + expect(files.get(at('list-orders/index.ts'))).toBe(MINE); + }); + + test('a manifest from before the `files` list still adopts the tree it generated', async () => { + const first = await runGen(); + const tree = Object.fromEntries(first.files); + // Strip the exact list, leaving the pre-guard shape (ownership graph only). + const legacy = JSON.parse( + tree[at('.stitch-gen.json')] as string, + ) as Record; + delete legacy['files']; + tree[at('.stitch-gen.json')] = JSON.stringify(legacy, null, 2); + + const { code, err } = await runGen([], tree); + + expect(code).toBe(0); + expect(err).not.toMatch(/refusing/); + }); + + test('--dry-run writes nothing and the guard never fires', async () => { + const { code, err, files } = await runGen(['--dry-run'], { + [SPEC_PATH]: SPEC, + [at('client.ts')]: MINE, + }); + + expect(code).toBe(0); + expect(err).not.toMatch(/refusing/); + expect([...files.keys()]).toEqual([SPEC_PATH, at('client.ts')]); + }); + + // End-to-end on a real directory: proves the default IO's existence probe sees a real file, so + // the guard is not an artefact of the fake filesystem above. + test('on a real directory, an author file survives the re-run untouched', async () => { + const dir = await mkdtemp(join(tmpdir(), 'stitch-openapi-')); + try { + await mkdir(join(dir, 'out'), { recursive: true }); + await writeFile(join(dir, 'openapi.json'), SPEC, 'utf8'); + await writeFile(join(dir, 'out', 'client.ts'), MINE, 'utf8'); + const err: string[] = []; + + const code = await main( + [join(dir, 'openapi.json'), '--all', '--out', join(dir, 'out')], + { write: () => undefined, writeErr: (s) => err.push(s) }, + ); + + expect(code).not.toBe(0); + expect(err.join('')).toMatch(/refusing to overwrite/); + expect(await readFile(join(dir, 'out', 'client.ts'), 'utf8')).toBe( + MINE, + ); + expect(await readdir(join(dir, 'out'))).toEqual(['client.ts']); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); +}); diff --git a/packages/openapi/test/gen-openapi.spec.ts b/packages/openapi/test/gen-openapi.spec.ts index 8dea0c8f..c648b8db 100644 --- a/packages/openapi/test/gen-openapi.spec.ts +++ b/packages/openapi/test/gen-openapi.spec.ts @@ -290,6 +290,28 @@ describe('planGen — naming, typing, auth, notice', () => { /runtime validation \+ drift are OFF/, ); }); + + // #694 §3: the warning is a build-time stderr line, the manifest is the DURABLE artefact. It + // recorded the requested tier, so a tree with zero validators claimed `"validator": "zod"`. + test('a requested tier that falls back records the EMITTED tier in the manifest', () => { + const r = planGen(doc, { all: true, validator: 'zod' }); + expect(r.warnings.join('\n')).toMatch( + /validator "zod" is not implemented in v1/, + ); + expect((r.manifest as { validator: string }).validator).toBe( + 'types-only', + ); + }); + + // The manifest's `files` list is what the CLI reads back to decide what it owns (#694 §1), so + // it has to be exact — every emitted path, including the manifest itself. + test('the manifest lists every path the run writes, itself included', () => { + const r = planGen(doc, { all: true }); + const listed = (r.manifest as { files: string[] }).files; + expect(listed).toEqual(r.files.map((f) => f.path).sort()); + expect(listed).toContain('.stitch-gen.json'); + expect(listed).toContain('client.ts'); + }); }); // The codegen turns an UNTRUSTED OpenAPI document into TS source the developer compiles. Spec text