From a00a9900a1c3dd0da6cfa5ab6cf81ace63394f37 Mon Sep 17 00:00:00 2001 From: Simo Kinnunen Date: Tue, 22 Sep 2026 01:05:34 +0900 Subject: [PATCH 1/4] feat(write-back): edit literal values inside a construct call [RED-985] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A source rewriter for the deploy write-back: `source-file.ts` parses a construct's file (acorn for JavaScript, typescript-estree for TypeScript and JSX) with its tokens and comments, and finds the one `new ('', { … })` whose class is bound by an import or require from a checkly package. `literal-edit.ts` resolves a property path inside that options literal, accepts only plain literals, renders JSON data in the file's own quote style, indentation, trailing-comma habit and line ending, and splices it in by byte range, so nothing else in the file changes. Commas are located through the token list, so a comma inside a comment cannot mislead a splice. Everything it cannot write without guessing is refused with a reason. Co-Authored-By: Claude Fable 5.1 --- .../write-back/__tests__/literal-edit.spec.ts | 463 +++++++++++++++ .../src/services/write-back/literal-edit.ts | 552 ++++++++++++++++++ .../src/services/write-back/source-file.ts | 257 ++++++++ 3 files changed, 1272 insertions(+) create mode 100644 packages/cli/src/services/write-back/__tests__/literal-edit.spec.ts create mode 100644 packages/cli/src/services/write-back/literal-edit.ts create mode 100644 packages/cli/src/services/write-back/source-file.ts diff --git a/packages/cli/src/services/write-back/__tests__/literal-edit.spec.ts b/packages/cli/src/services/write-back/__tests__/literal-edit.spec.ts new file mode 100644 index 00000000..1c472dde --- /dev/null +++ b/packages/cli/src/services/write-back/__tests__/literal-edit.spec.ts @@ -0,0 +1,463 @@ +import { describe, expect, it } from 'vitest' + +import { applyLiteralEdits, detectStyle, evaluateLiteral, isPlainLiteral, resolvePath } from '../literal-edit.js' +import { findConstructOptions, parseSource, WriteBackSkipped } from '../source-file.js' + +/** + * The rewriter (`source-file.ts`, `literal-edit.ts`): finding the construct + * call, deciding what may be replaced, and splicing rendered values in so + * that nothing else in the file moves. + */ + +const NAMES = new Set(['ApiCheck']) + +function options (file: string, text: string, logicalId = 'api') { + return findConstructOptions(parseSource(file, text), logicalId, NAMES) +} + +function edit (file: string, text: string, edits: { path: string[], value: unknown }[], logicalId = 'api') { + const source = parseSource(file, text) + return applyLiteralEdits(source, findConstructOptions(source, logicalId, NAMES), edits) +} + +/** Whether the edited text still parses and holds `value` at `path`, checked with the same parser. */ +function readsBack (file: string, text: string, path: string[], value: unknown): boolean { + const resolution = resolvePath(options(file, text), path) + return resolution.kind === 'found' && JSON.stringify(evaluateLiteral(resolution.node)) === JSON.stringify(value) +} + +const TS = `import { ApiCheck, Frequency } from 'checkly/constructs' + +const shared: string[] = ['eu-west-1'] + +new ApiCheck('api', { + name: 'API', + activated: true, + tags: ['a', 'b'], + locations: shared, + frequency: Frequency.EVERY_5M, + request: { + url: 'https://example.com', + method: 'GET', + headers: [ + { key: 'x-a', value: '1' }, + ], + }, + degradedResponseTime: 5000, +}) +` + +describe('findConstructOptions', () => { + it('finds the call through a named import, an alias and a require', () => { + expect(options('a.ts', TS).type).toBe('ObjectExpression') + const aliased = `import { ApiCheck as Api } from 'checkly'\nnew Api('api', { name: 'x' })\n` + expect(options('a.ts', aliased).properties).toHaveLength(1) + const required = `const { ApiCheck } = require('checkly/constructs')\nnew ApiCheck('api', { name: 'x' })\n` + expect(options('a.js', required).properties).toHaveLength(1) + const renamed = `const { ApiCheck: Api } = require('checkly/constructs')\nnew Api(\`api\`, { name: 'x' })\n` + expect(options('a.cjs', renamed).properties).toHaveLength(1) + }) + + it('does not match a class that is not imported from checkly', () => { + const own = `import { ApiCheck } from './my-checks'\nnew ApiCheck('api', { name: 'x' })\n` + expect(() => options('a.ts', own)).toThrow(/is not imported from checkly/) + const local = `class ApiCheck {}\nnew ApiCheck('api', { name: 'x' })\n` + expect(() => options('a.js', local)).toThrow(/is not imported from checkly/) + }) + + it('refuses a missing, duplicated or non-literal call', () => { + expect(() => options('a.ts', TS, 'other')).toThrow(/no `new ApiCheck\('other', …\)` found/) + const twice = `import { ApiCheck } from 'checkly/constructs'\nnew ApiCheck('api', { name: 'x' })\nnew ApiCheck('api', { name: 'y' })\n` + expect(() => options('a.ts', twice)).toThrow(/several constructs use the logical id 'api'/) + const variable = `import { ApiCheck } from 'checkly/constructs'\nconst opts = { name: 'x' }\nnew ApiCheck('api', opts)\n` + expect(() => options('a.ts', variable)).toThrow(/not a plain object literal/) + const spread = `import { ApiCheck } from 'checkly/constructs'\nnew ApiCheck('api', { ...base, name: 'x' })\n` + expect(() => options('a.ts', spread)).toThrow(/spread another object/) + const satisfies = `import { ApiCheck } from 'checkly/constructs'\nnew ApiCheck('api', { name: 'x' } satisfies object)\n` + expect(() => options('a.ts', satisfies)).toThrow(/not a plain object literal/) + const dynamicId = `import { ApiCheck } from 'checkly/constructs'\nfor (const id of ids) new ApiCheck(id, { name: 'x' })\n` + expect(() => options('a.ts', dynamicId)).toThrow(/no `new ApiCheck\('api', …\)` found/) + }) + + it('reads every supported extension and refuses others', () => { + const js = `const { ApiCheck } = require('checkly')\nnew ApiCheck('api', { name: 'x' })\n` + for (const file of ['a.js', 'a.mjs', 'a.cjs']) { + expect(options(file, js).properties).toHaveLength(1) + } + const ts = `import { ApiCheck } from 'checkly'\nnew ApiCheck('api', { name: 'x' as string })\n` + for (const file of ['a.ts', 'a.mts', 'a.cts', 'a.tsx']) { + expect(options(file, ts).properties).toHaveLength(1) + } + const esm = `import { ApiCheck } from 'checkly'\nexport const check = new ApiCheck('api', { name: 'x' })\n` + expect(options('a.mjs', esm).properties).toHaveLength(1) + expect(() => parseSource('a.json', '{}')).toThrow(/is not a JavaScript or TypeScript file/) + expect(() => parseSource('a.ts', 'new (')).toThrow(/could not parse the file/) + expect(() => parseSource('a.js', 'new (')).toThrow(WriteBackSkipped) + }) +}) + +describe('resolvePath', () => { + const node = options('a.ts', TS) + + it('finds plain literals at any depth', () => { + expect(resolvePath(node, ['name'])).toMatchObject({ kind: 'found' }) + expect(resolvePath(node, ['request', 'url'])).toMatchObject({ kind: 'found' }) + expect(resolvePath(node, ['request', 'headers'])).toMatchObject({ kind: 'found' }) + expect(resolvePath(node, ['request', 'headers', '0', 'value'])).toMatchObject({ kind: 'found' }) + }) + + it('reports a missing last key and refuses a missing parent', () => { + expect(resolvePath(node, ['muted'])).toMatchObject({ kind: 'missing', key: 'muted' }) + expect(resolvePath(node, ['request', 'body'])).toMatchObject({ kind: 'missing', key: 'body' }) + expect(resolvePath(node, ['heartbeat', 'period'])).toMatchObject({ kind: 'unsupported', reason: 'heartbeat is not set in the code' }) + }) + + it('refuses positions that are not plain literals', () => { + expect(resolvePath(node, ['frequency'])).toMatchObject({ kind: 'unsupported', reason: 'frequency is Frequency.EVERY_5M, not a plain literal' }) + expect(resolvePath(node, ['locations'])).toMatchObject({ kind: 'unsupported', reason: 'locations is the variable shared, not a plain literal' }) + expect(resolvePath(node, ['locations', '0'])).toMatchObject({ kind: 'unsupported', reason: 'locations is the variable shared, not an object literal' }) + const source = `import { ApiCheck } from 'checkly'\nconst name = 'x'\nnew ApiCheck('api', { name, description: null, tags: [\`\${a}\`], request: { url: url(), assertions: [AssertionBuilder.statusCode().equals(200)] }, twice: 1, twice: 2 })\n` + const dodgy = options('a.ts', source) + expect(resolvePath(dodgy, ['name'])).toMatchObject({ reason: 'name is the variable name, not a literal' }) + expect(resolvePath(dodgy, ['description'])).toMatchObject({ reason: 'description is null in the code; set a value by hand' }) + expect(resolvePath(dodgy, ['tags'])).toMatchObject({ reason: 'tags is an array with non-literal elements, not a plain literal' }) + expect(resolvePath(dodgy, ['request', 'url'])).toMatchObject({ reason: 'request.url is a function call, not a plain literal' }) + expect(resolvePath(dodgy, ['request', 'assertions'])).toMatchObject({ reason: /assertions is an array with non-literal elements/ }) + expect(resolvePath(dodgy, ['twice'])).toMatchObject({ reason: 'twice is set twice in the code' }) + const odd = `import { ApiCheck } from 'checkly'\nnew ApiCheck('api', { t: \`a\${b}\`, h: [1, , 2], r: /x/, request: { url: 'u', ...rest }, q: { url: 'u', ['url']: 'v' } })\n` + const node2 = options('a.ts', odd) + expect(resolvePath(node2, ['t', 'x'])).toMatchObject({ reason: 't is a template with expressions, not an object literal' }) + expect(resolvePath(node2, ['h'])).toMatchObject({ reason: 'h is an array with holes, not a plain literal' }) + expect(resolvePath(node2, ['r'])).toMatchObject({ reason: 'r is a regular expression, not a plain literal' }) + expect(resolvePath(node2, ['request', 'url'])).toMatchObject({ reason: 'url may be overridden by a later member' }) + expect(resolvePath(node2, ['q', 'url'])).toMatchObject({ reason: 'url may be overridden by a later member' }) + }) + + it('evaluates what it accepts', () => { + const source = `import { ApiCheck } from 'checkly'\nnew ApiCheck('api', { n: -1.5, s: \`plain\`, o: { 'a-b': [true, "x"] } })\n` + const node = options('a.ts', source) + for (const key of ['n', 's', 'o']) { + const found = resolvePath(node, [key]) + expect(found.kind).toBe('found') + expect(isPlainLiteral((found as { node: any }).node)).toBe(true) + } + expect(evaluateLiteral((resolvePath(node, ['n']) as any).node)).toBe(-1.5) + expect(evaluateLiteral((resolvePath(node, ['s']) as any).node)).toBe('plain') + expect(evaluateLiteral((resolvePath(node, ['o']) as any).node)).toEqual({ 'a-b': [true, 'x'] }) + }) +}) + +describe('detectStyle', () => { + const style = (file: string, text: string) => { + const source = parseSource(file, text) + return detectStyle(source, findConstructOptions(source, 'api', NAMES)) + } + + it('reads the quote, indent unit and line ending from the object', () => { + expect(style('a.ts', TS)).toEqual({ quote: '\'', indentUnit: ' ', lineEnding: '\n' }) + const tabs = `import { ApiCheck } from 'checkly'\r\nnew ApiCheck("api", {\r\n\tname: "x",\r\n\ttags: ["a", 'b'],\r\n})\r\n` + expect(style('a.ts', tabs)).toEqual({ quote: '"', indentUnit: '\t', lineEnding: '\r\n' }) + }) + + it('falls back to the rest of the file for the quote when the object has no strings', () => { + const inline = `import { ApiCheck } from "checkly"\nnew ApiCheck("api", { activated: true })\n` + expect(style('a.ts', inline)).toEqual({ quote: '"', indentUnit: ' ', lineEnding: '\n' }) + }) +}) + +describe('applyLiteralEdits', () => { + it('replaces scalars and arrays in place and leaves everything else byte-identical', () => { + const { text, applied, skipped } = edit('a.ts', TS, [ + { path: ['name'], value: 'API v2' }, + { path: ['activated'], value: false }, + { path: ['tags'], value: ['b', 'c', 'd'] }, + { path: ['request', 'url'], value: 'https://example.com/v2' }, + { path: ['degradedResponseTime'], value: 8000 }, + ]) + expect(skipped).toEqual([]) + expect(applied.map(a => [a.path.join('.'), a.previous, a.rendered])).toEqual([ + ['name', '\'API\'', '\'API v2\''], + ['activated', 'true', 'false'], + ['tags', '[\'a\', \'b\']', '[\'b\', \'c\', \'d\']'], + ['request.url', '\'https://example.com\'', '\'https://example.com/v2\''], + ['degradedResponseTime', '5000', '8000'], + ]) + expect(text).toBe(TS + .replace('\'API\'', '\'API v2\'') + .replace('activated: true', 'activated: false') + .replace('[\'a\', \'b\']', '[\'b\', \'c\', \'d\']') + .replace('https://example.com\'', 'https://example.com/v2\'') + .replace('5000', '8000')) + }) + + it('keeps a multi-line list multi-line, with the trailing comma the code used', () => { + const { text } = edit('a.ts', TS, [ + { path: ['request', 'headers'], value: [{ key: 'x-a', value: '2' }, { key: 'x-b', value: '3' }] }, + ]) + expect(text).toContain(` headers: [ + { + key: 'x-a', + value: '2', + }, + { + key: 'x-b', + value: '3', + }, + ], + },`) + }) + + it('inserts missing properties after the last member of a multi-line object', () => { + const { text, applied } = edit('a.ts', TS, [ + { path: ['muted'], value: true }, + { path: ['request', 'body'], value: '{"a":1}' }, + { path: ['request', 'queryParameters'], value: [{ key: 'q', value: 'v' }] }, + ]) + expect(applied.map(a => a.previous)).toEqual([undefined, undefined, undefined]) + expect(text).toContain(` degradedResponseTime: 5000, + muted: true, +})`) + expect(text).toContain(` headers: [ + { key: 'x-a', value: '1' }, + ], + body: '{"a":1}', + queryParameters: [ + { + key: 'q', + value: 'v', + }, + ], + },`) + }) + + it('adds a comma to a last member that has none, before a same-line comment', () => { + const source = `import { ApiCheck } from 'checkly' +new ApiCheck('api', { + name: 'x', // display name + activated: true // on +}) +` + const { text } = edit('a.ts', source, [{ path: ['muted'], value: false }, { path: ['tags'], value: ['t'] }]) + expect(text).toBe(`import { ApiCheck } from 'checkly' +new ApiCheck('api', { + name: 'x', // display name + activated: true, // on + muted: false, + tags: ['t'] +}) +`) + }) + + it('inserts into one-line and empty objects', () => { + const inline = `import { ApiCheck } from 'checkly'\nnew ApiCheck('api', { name: 'x' })\n` + expect(edit('a.ts', inline, [{ path: ['muted'], value: true }]).text) + .toBe(`import { ApiCheck } from 'checkly'\nnew ApiCheck('api', { name: 'x', muted: true })\n`) + const trailing = `import { ApiCheck } from 'checkly'\nnew ApiCheck('api', { name: 'x', })\n` + expect(edit('a.ts', trailing, [{ path: ['muted'], value: true }]).text) + .toBe(`import { ApiCheck } from 'checkly'\nnew ApiCheck('api', { name: 'x', muted: true, })\n`) + const empty = `import { ApiCheck } from 'checkly'\nnew ApiCheck('api', {})\n` + expect(edit('a.ts', empty, [{ path: ['name'], value: 'x' }]).text) + .toBe(`import { ApiCheck } from 'checkly'\nnew ApiCheck('api', { name: 'x' })\n`) + expect(edit('a.ts', empty, [{ path: ['request'], value: { url: 'u', method: 'GET' } }]).text) + .toBe(`import { ApiCheck } from 'checkly'\nnew ApiCheck('api', { request: { url: 'u', method: 'GET' } })\n`) + const commented = `import { ApiCheck } from 'checkly'\nnew ApiCheck('api', {\n // nothing yet\n})\n` + expect(edit('a.ts', commented, [{ path: ['name'], value: 'x' }, { path: ['tags'], value: [] }]).text) + .toBe(`import { ApiCheck } from 'checkly'\nnew ApiCheck('api', {\n name: 'x',\n tags: [],\n // nothing yet\n})\n`) + const padded = `import { ApiCheck } from 'checkly'\nnew ApiCheck('api', { /* none */ })\n` + expect(edit('a.ts', padded, [{ path: ['muted'], value: true }]).text) + .toBe(`import { ApiCheck } from 'checkly'\nnew ApiCheck('api', { muted: true, /* none */ })\n`) + const spaced = `import { ApiCheck } from 'checkly'\nnew ApiCheck('api', { })\n` + expect(edit('a.ts', spaced, [{ path: ['muted'], value: true }]).text) + .toBe(`import { ApiCheck } from 'checkly'\nnew ApiCheck('api', { muted: true })\n`) + const blankLines = `import { ApiCheck } from 'checkly'\nnew ApiCheck('api', {\n\n})\n` + expect(edit('a.ts', blankLines, [{ path: ['muted'], value: true }]).text) + .toBe(`import { ApiCheck } from 'checkly'\nnew ApiCheck('api', {\n muted: true\n})\n`) + }) + + it('follows double quotes, tabs and CRLF line endings', () => { + const source = `import { ApiCheck } from "checkly"\r\nnew ApiCheck("api", {\r\n\tname: "x",\r\n\trequest: {\r\n\t\turl: "u",\r\n\t},\r\n})\r\n` + const { text } = edit('a.ts', source, [ + { path: ['name'], value: 'it\'s "quoted"' }, + { path: ['tags'], value: [{ deep: 1 }] }, + ]) + expect(text).toBe(`import { ApiCheck } from "checkly"\r\nnew ApiCheck("api", {\r\n\tname: "it's \\"quoted\\"",\r\n\trequest: {\r\n\t\turl: "u",\r\n\t},\r\n\ttags: [\r\n\t\t{\r\n\t\t\tdeep: 1,\r\n\t\t},\r\n\t],\r\n})\r\n`) + }) + + it('encodes control characters and line separators so the file still parses', () => { + const value = 'a
b
c\nd\re\tf\\g\'h"i\0j' + const { text, applied } = edit('a.ts', TS, [{ path: ['name'], value }]) + expect(applied[0].rendered).toBe('\'a\\u2028b\\u2029c\\nd\\re\\tf\\\\g\\\'h"i\\u0000j\'') + const reread = resolvePath(options('a.ts', text), ['name']) + expect(reread.kind).toBe('found') + expect(evaluateLiteral((reread as any).node)).toBe(value) + // A backslash before a quote, and a lone surrogate, in both quote styles. + for (const tricky of ['\\\'', '\\"', 'x\\', '\ud800', '\'\\\'\'']) { + for (const file of [TS, TS.replace(/'/g, '"')]) { + const result = edit('a.ts', file, [{ path: ['name'], value: tricky }]) + expect(result.skipped).toEqual([]) + expect(readsBack('a.ts', result.text, ['name'], tricky)).toBe(true) + } + } + }) + + it('quotes keys that are not identifiers', () => { + const { text } = edit('a.ts', TS, [{ path: ['request', 'basicAuth'], value: { 'user-name': 'u', 'password': 'p' } }]) + expect(text).toContain(` basicAuth: { + 'user-name': 'u', + password: 'p', + }, + },`) + }) + + it('skips what it cannot write and says why', () => { + const { text, applied, skipped } = edit('a.ts', TS, [ + { path: ['frequency'], value: 10 }, + { path: ['locations'], value: ['us-east-1'] }, + { path: ['heartbeat', 'period'], value: 1 }, + { path: ['description'], value: null }, + { path: ['tags'], value: ['x', null] }, + { path: ['degradedResponseTime'], value: Number.POSITIVE_INFINITY }, + { path: ['name'], value: () => 1 }, + { path: ['request', 'url'], value: 'https://y' }, + { path: ['request'], value: { url: 'https://x' } }, + ]) + expect(applied.map(a => a.path.join('.'))).toEqual(['request.url']) + expect(skipped.map(s => [s.path.join('.'), s.reason])).toEqual([ + ['frequency', 'frequency is Frequency.EVERY_5M, not a plain literal'], + ['locations', 'locations is the variable shared, not a plain literal'], + ['heartbeat.period', 'heartbeat is not set in the code'], + ['description', 'Checkly has no value for description; edit the property by hand'], + ['tags', 'Checkly has no value for tags.1; edit the property by hand'], + ['degradedResponseTime', 'Infinity cannot be written as a literal'], + ['name', 'a function cannot be written as a literal'], + ['request', 'overlaps another edit'], + ]) + expect(text).toBe(TS.replace('https://example.com\'', 'https://y\'')) + }) + + it('is not fooled by commas or line breaks inside comments', () => { + const lineComment = `import { ApiCheck } from 'checkly' +new ApiCheck('api', { + name: 'x' // a, b +}) +` + let result = edit('a.ts', lineComment, [{ path: ['muted'], value: true }]) + expect(result.text).toBe(`import { ApiCheck } from 'checkly' +new ApiCheck('api', { + name: 'x', // a, b + muted: true +}) +`) + expect(readsBack('a.ts', result.text, ['muted'], true)).toBe(true) + + const blockComment = `import { ApiCheck } from 'checkly' +new ApiCheck('api', { + name: 'x', /* one, + two */ +}) +` + result = edit('a.ts', blockComment, [{ path: ['muted'], value: true }]) + expect(result.text).toBe(`import { ApiCheck } from 'checkly' +new ApiCheck('api', { + name: 'x', + muted: true, /* one, + two */ +}) +`) + expect(readsBack('a.ts', result.text, ['muted'], true)).toBe(true) + + const oneLine = `const { ApiCheck } = require('checkly')\nnew ApiCheck('api', { name: 'x' /* , */ })\n` + result = edit('a.js', oneLine, [{ path: ['muted'], value: true }]) + expect(result.text).toBe(`const { ApiCheck } = require('checkly')\nnew ApiCheck('api', { name: 'x', muted: true /* , */ })\n`) + expect(readsBack('a.js', result.text, ['muted'], true)).toBe(true) + + const ownLine = `import { ApiCheck } from 'checkly' +new ApiCheck('api', { + name: 'x' + , +}) +` + result = edit('a.ts', ownLine, [{ path: ['muted'], value: true }]) + expect(readsBack('a.ts', result.text, ['muted'], true)).toBe(true) + expect(readsBack('a.ts', result.text, ['name'], 'x')).toBe(true) + }) + + it('refuses to add a key next to members it cannot read', () => { + const source = `import { ApiCheck } from 'checkly'\nnew ApiCheck('api', { get name () { return 'x' }, ['muted']: true, tags: ['a'] })\n` + const { applied, skipped } = edit('a.ts', source, [ + { path: ['name'], value: 'y' }, + { path: ['muted'], value: false }, + { path: ['tags'], value: ['b'] }, + ]) + expect(skipped.map(s => s.reason)).toEqual([ + 'the options has members this tool cannot read', + 'the options has members this tool cannot read', + ]) + expect(applied.map(a => a.path.join('.'))).toEqual(['tags']) + }) + + it('refuses values that are not JSON data and quotes __proto__', () => { + // A `__proto__` key in a literal would set the prototype; `fromEntries` makes it an own property. + const proto = Object.fromEntries([['__proto__', 'x'], ['username', 'u']]) + const { text, skipped } = edit('a.ts', TS, [ + { path: ['name'], value: new Date(0) }, + { path: ['tags'], value: new Map() }, + { path: ['request', 'basicAuth'], value: proto }, + ]) + expect(skipped.map(s => [s.path.join('.'), s.reason])).toEqual([ + ['name', 'a Date cannot be written as a literal'], + ['tags', 'a Map cannot be written as a literal'], + ]) + expect(text).toContain(` basicAuth: { + '__proto__': 'x', + username: 'u', + },`) + expect(readsBack('a.ts', text, ['request', 'basicAuth'], proto)).toBe(true) + }) + + it('reads .jsx and .tsx through the TypeScript parser', () => { + const jsx = `import { ApiCheck } from 'checkly'\nconst el =
\nnew ApiCheck('api', { name: 'x' })\n` + for (const file of ['a.jsx', 'a.tsx']) { + expect(edit(file, jsx, [{ path: ['muted'], value: true }]).text).toContain(`{ name: 'x', muted: true }`) + } + }) + + it('refuses an empty path and keeps a member on the last line at its own column', () => { + expect(edit('a.ts', TS, [{ path: [], value: {} }]).skipped).toEqual([{ path: [], value: {}, reason: 'no property named' }]) + const cramped = `import { ApiCheck } from 'checkly' +new ApiCheck('api', { name: 'x', tags: [ + 'a', +] }) +` + const { text } = edit('a.ts', cramped, [{ path: ['muted'], value: true }]) + expect(text).toBe(`import { ApiCheck } from 'checkly' +new ApiCheck('api', { name: 'x', tags: [ + 'a', +], + muted: true }) +`) + expect(readsBack('a.ts', text, ['muted'], true)).toBe(true) + }) + + it('applies only the first of two edits to the same bytes', () => { + const source = `import { ApiCheck } from 'checkly'\nnew ApiCheck('api', { request: { headers: [{ key: 'a', value: '1' }] } })\n` + const { text, skipped } = edit('a.ts', source, [ + { path: ['request', 'headers'], value: [{ key: 'b', value: '2' }] }, + { path: ['request', 'headers', '0', 'value'], value: '3' }, + { path: ['muted'], value: true }, + { path: ['muted'], value: false }, + ]) + expect(skipped.map(s => [s.path.join('.'), s.reason])).toEqual([ + ['request.headers.0.value', 'overlaps another edit'], + ['muted', 'overlaps another edit'], + ]) + expect(text).toBe(`import { ApiCheck } from 'checkly'\nnew ApiCheck('api', { request: { headers: [{ key: 'b', value: '2' }] }, muted: true })\n`) + // An insertion into an object that another edit replaces whole. + const nested = edit('a.ts', source, [ + { path: ['request'], value: { url: 'u' } }, + { path: ['request', 'body'], value: 'b' }, + ]) + expect(nested.skipped).toEqual([{ path: ['request', 'body'], value: 'b', reason: 'overlaps another edit' }]) + expect(readsBack('a.ts', nested.text, ['request'], { url: 'u' })).toBe(true) + }) +}) diff --git a/packages/cli/src/services/write-back/literal-edit.ts b/packages/cli/src/services/write-back/literal-edit.ts new file mode 100644 index 00000000..e7c00115 --- /dev/null +++ b/packages/cli/src/services/write-back/literal-edit.ts @@ -0,0 +1,552 @@ +import type { TSESTree } from '@typescript-eslint/typescript-estree' +import { type Node, type ParsedSource, type SourceToken, WriteBackSkipped, walk } from './source-file.js' + +/** + * Splices literal values into the options object of a construct call. + * + * Every edit names a path inside the object (`['request', 'url']`) and the + * value that position should hold. A position that exists and holds a plain + * literal is replaced; a key the innermost object lacks is inserted after + * the object's last member; anything else — a `Frequency.*` constant, a + * variable, a spread, a missing parent — is refused, because a rewrite that + * guesses what the code means is worse than none. + * + * The file is edited by byte range, so nothing outside the touched values + * changes. The rendering of a new value copies what surrounds it: the quote + * character the object already uses, its indentation unit, whether it ends + * members with a trailing comma, and the file's line ending. Commas and + * comments are located through the parser's tokens, so a comma or a line + * break inside a comment cannot mislead a splice. (`src/sourcegen` renders construct source too, but it + * orders keys and fixes the style, which is what a splice into a user's + * file must not do.) + */ + +export interface LiteralEdit { + /** Property path inside the options object; a numeric segment indexes an array. */ + path: string[] + /** The value to write. `null` and `undefined` are refused (see `renderValue`). */ + value: unknown +} + +export interface AppliedEdit extends LiteralEdit { + /** The source text the edit replaced, or undefined when the property was added. */ + previous?: string + /** The text written for the value. */ + rendered: string +} + +export interface SkippedEdit extends LiteralEdit { + reason: string +} + +export interface EditResult { + text: string + applied: AppliedEdit[] + skipped: SkippedEdit[] +} + +export interface SourceStyle { + quote: '\'' | '"' + indentUnit: string + lineEnding: '\n' | '\r\n' +} + +type ObjectNode = TSESTree.ObjectExpression +type PropertyNode = TSESTree.Property + +type Resolution = + | { kind: 'found', node: Node } + | { kind: 'missing', parent: ObjectNode, key: string } + | { kind: 'unsupported', reason: string } + +const IDENTIFIER = /^[A-Za-z_$][\w$]*$/ + +/** The name of a plain `key: value` member, or undefined for a spread, method, accessor or computed key. */ +function memberName (property: PropertyNode | TSESTree.SpreadElement): string | undefined { + if (property.type !== 'Property' || property.computed || property.kind !== 'init' || property.method) { + return undefined + } + if (property.key.type === 'Identifier') { + return property.key.name + } + if (property.key.type === 'Literal' && typeof property.key.value === 'string') { + return property.key.value + } + return undefined +} + +/** + * Whether a node is a value this module could have written itself: a + * string, number or boolean literal, a template with no `${}`, a negated + * number, or an array or object built only of those. Such a node can be + * replaced wholesale without losing anything but the comments inside it. + */ +export function isPlainLiteral (node: Node): boolean { + switch (node.type) { + case 'Literal': + return typeof node.value === 'string' || typeof node.value === 'number' || typeof node.value === 'boolean' + case 'TemplateLiteral': + return node.expressions.length === 0 + case 'UnaryExpression': + return node.operator === '-' && node.argument.type === 'Literal' && typeof node.argument.value === 'number' + case 'ArrayExpression': + return node.elements.every(element => element !== null && element.type !== 'SpreadElement' && isPlainLiteral(element)) + case 'ObjectExpression': + return node.properties.every(property => + memberName(property) !== undefined + && !(property as PropertyNode).shorthand + && isPlainLiteral((property as PropertyNode).value)) + default: + return false + } +} + +/** The value a plain literal node evaluates to; only meaningful when `isPlainLiteral` holds. */ +export function evaluateLiteral (node: Node): unknown { + switch (node.type) { + case 'Literal': + return node.value + case 'TemplateLiteral': + return node.quasis.map(quasi => quasi.value.cooked ?? '').join('') + case 'UnaryExpression': + return -(evaluateLiteral(node.argument) as number) + case 'ArrayExpression': + return node.elements.map(element => (element === null ? undefined : evaluateLiteral(element))) + case 'ObjectExpression': + return Object.fromEntries(node.properties.map(property => + [memberName(property), evaluateLiteral((property as PropertyNode).value)])) + default: + return undefined + } +} + +function describe (node: Node): string { + switch (node.type) { + case 'Identifier': + return `the variable ${node.name}` + case 'MemberExpression': + return node.object.type === 'Identifier' && node.property.type === 'Identifier' + ? `${node.object.name}.${node.property.name}` + : 'a member expression' + case 'CallExpression': + return 'a function call' + case 'NewExpression': + return 'a constructor call' + case 'TemplateLiteral': + return node.expressions.length === 0 ? 'a string' : 'a template with expressions' + case 'Literal': + if (node.value === null) { + return 'null' + } + if ('regex' in node && node.regex !== undefined) { + return 'a regular expression' + } + return typeof node.value === 'bigint' ? 'a bigint' : 'a literal' + case 'ArrayExpression': + return node.elements.some(element => element === null) ? 'an array with holes' : 'an array with non-literal elements' + case 'ObjectExpression': + return 'an object with non-literal members' + default: + return node.type.startsWith('TS') ? 'a TypeScript expression' : 'not a plain literal' + } +} + +/** + * Where `path` lands inside `options`: an existing plain literal, a key the + * innermost object lacks, or a position this module must not touch. + */ +export function resolvePath (options: ObjectNode, path: readonly string[]): Resolution { + if (path.length === 0) { + return { kind: 'unsupported', reason: 'no property named' } + } + let node: Node = options + for (let i = 0; i < path.length; i++) { + const segment = path[i] + const last = i === path.length - 1 + if (node.type === 'ObjectExpression') { + const members = node.properties.filter(property => memberName(property) === segment) as PropertyNode[] + if (members.length > 1) { + return { kind: 'unsupported', reason: `${segment} is set twice in the code` } + } + if (members.length === 0) { + if (!last) { + return { kind: 'unsupported', reason: `${path.slice(0, i + 1).join('.')} is not set in the code` } + } + // A method, accessor or computed key could be this very property + // under a spelling this module cannot read; adding a second one + // would leave the object with two. + if (node.properties.some(property => memberName(property) === undefined)) { + const where = path.slice(0, i).join('.') || 'the options' + return { kind: 'unsupported', reason: `${where} has members this tool cannot read` } + } + return { kind: 'missing', parent: node, key: segment } + } + if (members[0].shorthand) { + return { kind: 'unsupported', reason: `${segment} is the variable ${segment}, not a literal` } + } + // A spread or an unreadable member after it could override the value + // at runtime, which would make the edit look applied and change nothing. + const index = node.properties.indexOf(members[0]) + if (node.properties.slice(index + 1).some(property => memberName(property) === undefined)) { + return { kind: 'unsupported', reason: `${segment} may be overridden by a later member` } + } + node = members[0].value + } else if (node.type === 'ArrayExpression') { + const index = /^\d+$/.test(segment) ? Number(segment) : -1 + const element: Node | null | undefined = node.elements[index] + if (element === null || element === undefined) { + return { kind: 'unsupported', reason: `${path.slice(0, i + 1).join('.')} is not set in the code` } + } + node = element + } else { + return { kind: 'unsupported', reason: `${path.slice(0, i).join('.')} is ${describe(node)}, not an object literal` } + } + } + if (node.type === 'Literal' && node.value === null) { + return { kind: 'unsupported', reason: `${path.join('.')} is null in the code; set a value by hand` } + } + if (!isPlainLiteral(node)) { + return { kind: 'unsupported', reason: `${path.join('.')} is ${describe(node)}, not a plain literal` } + } + return { kind: 'found', node } +} + +/** + * The conventions the edited object follows: the quote most of its strings + * use (the whole file's when it has none), the indentation unit between it + * and its members, and the file's line ending. Defaults (single quotes, two + * spaces, LF) apply where there is no evidence. + */ +export function detectStyle (source: ParsedSource, options: ObjectNode): SourceStyle { + const { text } = source + const count = (root: Node) => { + let single = 0 + let double = 0 + for (const node of walk(root)) { + if (node.type === 'Literal' && typeof node.value === 'string') { + if (text[node.range[0]] === '"') { + double++ + } else { + single++ + } + } + } + return { single, double } + } + let quotes = count(options) + if (quotes.single === 0 && quotes.double === 0) { + quotes = count(source.program) + } + const lineEnding = text.includes('\r\n') ? '\r\n' : '\n' + let indentUnit = ' ' + const first = options.properties[0] + if (first !== undefined && isMultiLine(text, options)) { + const outer = indentationAt(text, options.range[0]) + const inner = indentationAt(text, first.range[0]) + if (inner.length > outer.length && inner.startsWith(outer)) { + indentUnit = inner.slice(outer.length) + } + } + return { quote: quotes.double > quotes.single ? '"' : '\'', indentUnit, lineEnding } +} + +function lineStart (text: string, offset: number): number { + const index = text.lastIndexOf('\n', offset - 1) + return index === -1 ? 0 : index + 1 +} + +/** The leading whitespace of the line holding `offset`. */ +function indentationAt (text: string, offset: number): string { + const start = lineStart(text, offset) + const match = /^[ \t]*/.exec(text.slice(start, offset)) + return match === null ? '' : match[0] +} + +function isMultiLine (text: string, node: Node): boolean { + return text.slice(node.range[0], node.range[1]).includes('\n') +} + +/** The tokens and comments lying within `[start, end)`. */ +function tokensBetween (source: ParsedSource, start: number, end: number): SourceToken[] { + return source.tokens.filter(token => token.range[0] >= start && token.range[1] <= end) +} + +/** The comma token that follows a list's last member, if the list has one. */ +function trailingCommaOf (source: ParsedSource, list: ObjectNode | TSESTree.ArrayExpression): SourceToken | undefined { + const members = list.type === 'ObjectExpression' ? list.properties : list.elements + const last = members[members.length - 1] + if (last === null || last === undefined) { + return undefined + } + return tokensBetween(source, last.range[1], list.range[1] - 1).find(token => token.kind === 'token' && token.value === ',') +} + +function quoteString (value: string, quote: SourceStyle['quote']): string { + // JSON.stringify encodes every control character and backslash; U+2028 + // and U+2029 it leaves in the clear, and a parser reading them as line + // terminators would see an unterminated string. Only the quote character + // needs swapping for a single-quoted file. + const json = JSON.stringify(value).replace(/\u2028/g, '\\u2028').replace(/\u2029/g, '\\u2029') + if (quote === '"') { + return json + } + return `'${json.slice(1, -1).replace(/\\"/g, '"').replace(/'/g, '\\\'')}'` +} + +function renderKey (key: string, quote: SourceStyle['quote']): string { + // A bare `__proto__` in an object literal sets the prototype rather than + // a property; quoted, it is a property like any other. + return IDENTIFIER.test(key) && key !== '__proto__' ? key : quoteString(key, quote) +} + +function isScalar (value: unknown): boolean { + return typeof value === 'string' || typeof value === 'number' || typeof value === 'boolean' +} + +function isPlainObject (value: unknown): value is Record { + if (typeof value !== 'object' || value === null || Array.isArray(value)) { + return false + } + const prototype = Object.getPrototypeOf(value) + return prototype === Object.prototype || prototype === null +} + +/** + * The source text for a value. Arrays of scalars stay on one line; arrays + * holding objects, and objects, put one member per line indented one unit + * past `column` (the indentation of the line the value starts on), unless + * `inline` asks for the one-line form an existing node used. `trailingComma` + * ends the last member of a multi-line list with a comma, as the edited + * object does. + * + * @throws WriteBackSkipped for a value the construct cannot hold as a + * literal: null or undefined (Checkly has cleared the property, and removing + * it from the code would hand it to a default that need not match), a + * non-finite number, or anything that is not JSON data. + */ +export function renderValue ( + value: unknown, + style: SourceStyle, + layout: { column: string, inline: boolean, trailingComma: boolean, at?: string }, +): string { + if (value === null || value === undefined) { + throw new WriteBackSkipped(layout.at === undefined + ? 'Checkly has no value for it; edit the property by hand' + : `Checkly has no value for ${layout.at}; edit the property by hand`) + } + if (typeof value === 'string') { + return quoteString(value, style.quote) + } + if (typeof value === 'number') { + if (!Number.isFinite(value)) { + throw new WriteBackSkipped(`${value} cannot be written as a literal`) + } + return String(value) + } + if (typeof value === 'boolean') { + return String(value) + } + const inner = layout.column + style.indentUnit + const nested = (member: unknown, key: string) => + renderValue(member, style, { ...layout, column: inner, at: layout.at === undefined ? key : `${layout.at}.${key}` }) + const block = (open: string, members: string[], close: string) => + `${open}${style.lineEnding}${members.map(member => `${inner}${member}`).join(`,${style.lineEnding}`)}` + + `${layout.trailingComma ? ',' : ''}${style.lineEnding}${layout.column}${close}` + if (Array.isArray(value)) { + if (value.length === 0) { + return '[]' + } + const members = value.map((member, index) => nested(member, String(index))) + return layout.inline || value.every(isScalar) ? `[${members.join(', ')}]` : block('[', members, ']') + } + if (isPlainObject(value)) { + const entries = Object.entries(value) + if (entries.length === 0) { + return '{}' + } + const members = entries.map(([key, member]) => `${renderKey(key, style.quote)}: ${nested(member, key)}`) + return layout.inline ? `{ ${members.join(', ')} }` : block('{', members, '}') + } + const kind = typeof value === 'object' ? (value.constructor?.name ?? 'object') : typeof value + throw new WriteBackSkipped(`a ${kind} cannot be written as a literal`) +} + +interface Splice { + start: number + end: number + text: string +} + +/** + * Applies `edits` to the text of `source` inside `options`, one of its + * nodes. Edits are resolved against the original ranges and spliced from + * the end of the file backwards, so no edit shifts another. Two edits that + * touch the same bytes cannot both be right: a replacement wins over an + * insertion into the object it replaces, and otherwise the first listed + * wins; the loser is skipped. Replacements are reported before insertions. + * The result is text only: to edit it again, parse it again. + */ +export function applyLiteralEdits ( + source: ParsedSource, + options: ObjectNode, + edits: readonly LiteralEdit[], +): EditResult { + const { text } = source + const style = detectStyle(source, options) + const applied: AppliedEdit[] = [] + const skipped: SkippedEdit[] = [] + const splices: Splice[] = [] + const insertions = new Map() + + const claim = (splice: Splice): boolean => { + const clash = splices.some(other => splice.start < other.end && other.start < splice.end) + if (!clash) { + splices.push(splice) + } + return !clash + } + + for (const edit of edits) { + try { + const resolution = resolvePath(options, edit.path) + if (resolution.kind === 'unsupported') { + skipped.push({ ...edit, reason: resolution.reason }) + continue + } + if (resolution.kind === 'found') { + const { node } = resolution + const rendered = renderValue(edit.value, style, { + at: edit.path.join('.'), + column: indentationAt(text, node.range[0]), + inline: !isMultiLine(text, node), + trailingComma: (node.type === 'ArrayExpression' || node.type === 'ObjectExpression') + && trailingCommaOf(source, node) !== undefined, + }) + if (!claim({ start: node.range[0], end: node.range[1], text: rendered })) { + skipped.push({ ...edit, reason: 'overlaps another edit' }) + continue + } + applied.push({ ...edit, previous: text.slice(node.range[0], node.range[1]), rendered }) + continue + } + const { parent, key } = resolution + const rendered = renderValue(edit.value, style, { + at: edit.path.join('.'), + column: memberColumn(text, parent, style), + inline: !isMultiLine(text, parent), + trailingComma: trailingCommaOf(source, parent) !== undefined, + }) + const pending = insertions.get(parent) ?? [] + if (pending.some(other => other.key === key)) { + skipped.push({ ...edit, reason: 'overlaps another edit' }) + continue + } + pending.push({ key, rendered, edit }) + insertions.set(parent, pending) + } catch (err) { + if (err instanceof WriteBackSkipped) { + skipped.push({ ...edit, reason: err.message }) + continue + } + throw err + } + } + + for (const [parent, pending] of insertions) { + const splice = insertionSplice(text, source, parent, pending, style) + if (!claim(splice)) { + for (const { edit } of pending) { + skipped.push({ ...edit, reason: 'overlaps another edit' }) + } + continue + } + for (const { edit, rendered } of pending) { + applied.push({ ...edit, rendered }) + } + } + + splices.sort((a, b) => b.start - a.start) + let result = text + for (const splice of splices) { + result = result.slice(0, splice.start) + splice.text + result.slice(splice.end) + } + return { text: result, applied, skipped } +} + +/** + * The indentation a member added to `parent` gets: that of the last + * member's line when it starts one, otherwise one unit past the object's + * own line (which also covers an empty object). + */ +function memberColumn (text: string, parent: ObjectNode, style: SourceStyle): string { + const last = parent.properties[parent.properties.length - 1] + if (last !== undefined) { + const indentation = indentationAt(text, last.range[0]) + if (lineStart(text, last.range[0]) + indentation.length === last.range[0]) { + return indentation + } + } + return indentationAt(text, parent.range[0]) + style.indentUnit +} + +/** + * The single splice that adds `pending` members to `parent`, after its last + * member. A multi-line object gets one line per member at the last member's + * indentation, a trailing comma on the last new member only if the object + * already used one, and a comma added right after the old last member if it + * had none. The new lines go after the line the last member (or its comma) + * ends on, so a comment on that line stays with that member — unless a + * comment runs on past that line end, in which case they go right after the + * member's comma rather than into the comment. A one-line object gets + * `, key: value` before its closing brace, and an empty one is rewritten as + * `{ key: value }`. + */ +function insertionSplice ( + text: string, + source: ParsedSource, + parent: ObjectNode, + pending: readonly { key: string, rendered: string }[], + style: SourceStyle, +): Splice { + const members = pending.map(({ key, rendered }) => `${renderKey(key, style.quote)}: ${rendered}`) + const last = parent.properties[parent.properties.length - 1] + const closing = parent.range[1] - 1 + if (last === undefined) { + // Between the braces, so a comment inside `{ }` stays; the braces and + // whatever they hold are not replaced. + const at = parent.range[0] + 1 + // Whitespace alone between the braces is replaced; a comment stays, + // after the new members. + const held = tokensBetween(source, at, closing).length > 0 + const end = held ? at : closing + if (!isMultiLine(text, parent)) { + return { start: at, end, text: ` ${members.join(', ')}${held ? ',' : ' '}` } + } + const inner = indentationAt(text, parent.range[0]) + style.indentUnit + const lines = members.map(member => `${style.lineEnding}${inner}${member}`).join(',') + const close = held ? ',' : `${style.lineEnding}${indentationAt(text, parent.range[0])}` + return { start: at, end, text: `${lines}${close}` } + } + const comma = trailingCommaOf(source, parent) + const afterMember = comma === undefined ? last.range[1] : comma.range[1] + if (!isMultiLine(text, parent)) { + // `{ a: 1 }` or `{ a: 1, }`: the new members join the line. + const text = `${comma === undefined ? ',' : ''} ${members.join(', ')}${comma === undefined ? '' : ','}` + return { start: afterMember, end: afterMember, text } + } + const breakAt = text.indexOf('\n', afterMember) + const lineEnd = breakAt === -1 || breakAt >= closing + ? afterMember + : text[breakAt - 1] === '\r' ? breakAt - 1 : breakAt + const straddles = tokensBetween(source, last.range[1], closing) + .some(token => token.range[0] < lineEnd && token.range[1] > lineEnd) + const at = straddles ? afterMember : lineEnd + const column = memberColumn(text, parent, style) + const lines = members.map(member => `${style.lineEnding}${column}${member}`).join(',') + if (comma === undefined) { + // The comma goes on the member; whatever sat between it and the line + // end (a comment) is kept, and the new lines follow. + return { start: last.range[1], end: at, text: `,${text.slice(last.range[1], at)}${lines}` } + } + return { start: at, end: at, text: `${lines},` } +} diff --git a/packages/cli/src/services/write-back/source-file.ts b/packages/cli/src/services/write-back/source-file.ts new file mode 100644 index 00000000..22894641 --- /dev/null +++ b/packages/cli/src/services/write-back/source-file.ts @@ -0,0 +1,257 @@ +import path from 'node:path' +import { createRequire } from 'node:module' +import * as acorn from 'acorn' +import type { TSESTree } from '@typescript-eslint/typescript-estree' + +/** + * Reads a construct's source file far enough to find the `new X('id', { … })` + * that declared it, so `literal-edit.ts` can splice values into that options + * object by byte range. + * + * Two parsers, one AST shape: `.js`/`.mjs`/`.cjs` go through acorn, which the + * CLI always ships; TypeScript and JSX go through typescript-estree, which + * needs the project's own `typescript` — the same requirement the check + * dependency parser already places on TypeScript check files. Both produce + * ESTree nodes carrying `range`, and only `range` and the node types below are + * read. Recast (which edits `checkly.config.ts` elsewhere in the CLI) is not + * used: its acorn parser cannot read TypeScript, and reprinting a touched + * node re-quotes it, whereas splicing by range leaves every other byte of the + * file exactly as the user wrote it. + */ + +const SOURCE_EXTENSIONS: ReadonlySet = new Set(['.js', '.mjs', '.cjs', '.jsx', '.ts', '.mts', '.cts', '.tsx']) + +/** Why a file, a construct or a value cannot be edited; the message is shown to the user as the reason it was skipped. */ +export class WriteBackSkipped extends Error {} + +export type Node = TSESTree.Node + +/** A token or a comment of the parsed file: only its kind and where it sits are read. */ +export interface SourceToken { + kind: 'token' | 'comment' + /** The token's text; empty for a comment. */ + value: string + range: [number, number] +} + +/** + * A parsed file with its tokens and comments, which the splicer needs to + * tell a comma of the source from one inside a comment. + */ +export interface ParsedSource { + /** The text the program was parsed from; every range below indexes it. */ + text: string + program: TSESTree.Program + tokens: SourceToken[] +} + +const CHECKLY_MODULE = /^checkly(\/.*)?$/ + +let tsParser: typeof import('@typescript-eslint/typescript-estree') | undefined + +function loadTsParser (): typeof import('@typescript-eslint/typescript-estree') { + if (tsParser !== undefined) { + return tsParser + } + try { + const require = createRequire(import.meta.url) + tsParser = require('@typescript-eslint/typescript-estree') + return tsParser as typeof import('@typescript-eslint/typescript-estree') + } catch (err: any) { + if (err.code === 'ERR_MODULE_NOT_FOUND' || err.code === 'MODULE_NOT_FOUND') { + throw new WriteBackSkipped('install "typescript" in the project to update TypeScript and JSX files') + } + throw err + } +} + +export function parseSource (filePath: string, text: string): ParsedSource { + const extension = path.extname(filePath) + if (!SOURCE_EXTENSIONS.has(extension)) { + throw new WriteBackSkipped(`${extension === '' ? 'the file' : `"${extension}"`} is not a JavaScript or TypeScript file`) + } + try { + if (extension === '.js' || extension === '.mjs' || extension === '.cjs') { + const tokens: SourceToken[] = [] + // The same options the check dependency parser reads .js files with, + // so the two agree on what parses. + const program = acorn.parse(text, { + ecmaVersion: 'latest', + ranges: true, + allowHashBang: true, + allowImportExportEverywhere: true, + allowReturnOutsideFunction: true, + allowAwaitOutsideFunction: true, + onToken: token => { + if (token.type.label !== 'eof') { + tokens.push({ kind: 'token', value: text.slice(token.start, token.end), range: [token.start, token.end] }) + } + }, + onComment: (_block, _text, start, end) => tokens.push({ kind: 'comment', value: '', range: [start, end] }), + }) as unknown as TSESTree.Program + return { text, program, tokens: tokens.sort((a, b) => a.range[0] - b.range[0]) } + } + const program = loadTsParser().parse(text, { + range: true, + comment: true, + tokens: true, + jsx: extension.endsWith('x'), + }) + const tokens: SourceToken[] = [ + ...(program.tokens ?? []).map((token): SourceToken => ({ kind: 'token', value: token.value, range: token.range })), + ...(program.comments ?? []).map((comment): SourceToken => ({ kind: 'comment', value: '', range: comment.range })), + ] + return { text, program, tokens: tokens.sort((a, b) => a.range[0] - b.range[0]) } + } catch (err: any) { + if (err instanceof WriteBackSkipped) { + throw err + } + throw new WriteBackSkipped(`could not parse the file: ${err.message}`) + } +} + +/** Every node below (and including) `root`, in source order. Only object-valued keys are followed, which covers every ESTree child slot. */ +export function* walk (root: Node): Generator { + const stack: Node[] = [root] + while (stack.length > 0) { + const node = stack.pop() as Node + yield node + const children: Node[] = [] + for (const [key, value] of Object.entries(node)) { + // `parent` would cycle; a program's token and comment lists are not + // part of the tree. + if (key === 'parent' || key === 'tokens' || key === 'comments') { + continue + } + if (Array.isArray(value)) { + for (const element of value) { + if (isNode(element)) { + children.push(element) + } + } + } else if (isNode(value)) { + children.push(value) + } + } + for (let i = children.length - 1; i >= 0; i--) { + stack.push(children[i]) + } + } +} + +function isNode (value: unknown): value is Node { + return typeof value === 'object' && value !== null && typeof (value as { type?: unknown }).type === 'string' +} + +/** The string a literal argument spells, for a plain string literal or a template with no `${}` in it. */ +function stringOf (node: Node | null | undefined): string | undefined { + if (node === null || node === undefined) { + return undefined + } + if (node.type === 'Literal' && typeof node.value === 'string') { + return node.value + } + if (node.type === 'TemplateLiteral' && node.expressions.length === 0 && node.quasis.length === 1) { + return node.quasis[0].value.cooked ?? undefined + } + return undefined +} + +/** + * The local names this file binds to the given exported names of a checkly + * package: `import { ApiCheck } from 'checkly/constructs'`, + * `import { ApiCheck as Api } from 'checkly'`, and the top-level + * `const { ApiCheck } = require('checkly/constructs')` (with or without a + * rename). Anything bound another way — a namespace import, a re-export from + * the user's own module, a wrapper class — is not a construct call this + * module can recognise, which keeps it from editing the options of a class + * that merely shares the name. Scope is not tracked: a local that shadows + * the import inside a function is taken for it, which the logical id and + * the later read-back of the edit bound. + */ +function checklyBindings (program: TSESTree.Program, exportedNames: ReadonlySet): Set { + const locals = new Set() + for (const statement of program.body) { + if (statement.type === 'ImportDeclaration') { + if (typeof statement.source.value !== 'string' || !CHECKLY_MODULE.test(statement.source.value)) { + continue + } + for (const specifier of statement.specifiers) { + if (specifier.type !== 'ImportSpecifier') { + continue + } + const imported = specifier.imported.type === 'Identifier' ? specifier.imported.name : stringOf(specifier.imported) + if (imported !== undefined && exportedNames.has(imported)) { + locals.add(specifier.local.name) + } + } + } else if (statement.type === 'VariableDeclaration') { + for (const declarator of statement.declarations) { + if (declarator.id.type !== 'ObjectPattern' || !isChecklyRequire(declarator.init)) { + continue + } + for (const property of declarator.id.properties) { + if (property.type !== 'Property' || property.computed || property.value.type !== 'Identifier') { + continue + } + const imported = property.key.type === 'Identifier' ? property.key.name : stringOf(property.key) + if (imported !== undefined && exportedNames.has(imported)) { + locals.add(property.value.name) + } + } + } + } + } + return locals +} + +function isChecklyRequire (node: Node | null | undefined): boolean { + return node !== null && node !== undefined + && node.type === 'CallExpression' + && node.callee.type === 'Identifier' && node.callee.name === 'require' + && node.arguments.length === 1 + && CHECKLY_MODULE.test(stringOf(node.arguments[0]) ?? '') +} + +/** + * The options object literal of the one `new ('', { … })` + * in the file, where `` is bound to one of `exportedNames` from a + * checkly package. + * + * @throws WriteBackSkipped when the file has no such call, several of them, + * or its options are not a plain object literal (a variable, a spread, a call, + * a TypeScript `as`/`satisfies` wrapper): anything the rewriter cannot edit + * without guessing what the code means. + */ +export function findConstructOptions ( + { program }: ParsedSource, + logicalId: string, + exportedNames: ReadonlySet, +): TSESTree.ObjectExpression { + const locals = checklyBindings(program, exportedNames) + const names = [...exportedNames].sort().join(' or ') + if (locals.size === 0) { + throw new WriteBackSkipped(`${names} is not imported from checkly in this file`) + } + const matches: TSESTree.NewExpression[] = [] + for (const node of walk(program)) { + if (node.type === 'NewExpression' && node.callee.type === 'Identifier' && locals.has(node.callee.name) + && stringOf(node.arguments[0]) === logicalId) { + matches.push(node) + } + } + if (matches.length === 0) { + throw new WriteBackSkipped(`no \`new ${[...locals].sort().join('|')}('${logicalId}', …)\` found in this file`) + } + if (matches.length > 1) { + throw new WriteBackSkipped(`several constructs use the logical id '${logicalId}' in this file`) + } + const options = matches[0].arguments[1] + if (options === undefined || options.type !== 'ObjectExpression') { + throw new WriteBackSkipped('its options are not a plain object literal') + } + if (options.properties.some(property => property.type === 'SpreadElement')) { + throw new WriteBackSkipped('its options spread another object') + } + return options +} From dfab2a38bd875f671b0900d05dda0cefe500e759 Mon Sep 17 00:00:00 2001 From: Simo Kinnunen Date: Tue, 22 Sep 2026 02:00:37 +0900 Subject: [PATCH 2/4] feat(write-back): plan which remote changes can be written into source [RED-985] `planWriteBack` turns the changes a deploy plan attributes to the Checkly account (`origin: 'remote'` or `'both'`) into literal edits of the construct files, and `applyWriteBack` writes them through a temporary file renamed into place. A per-class table names the properties whose import-format spelling and construct spelling are both literals; values come from the entry's full-detail `before`. Refused with a reason: references, secrets and anything the redaction table blanks, hashed or masked values, CLI-spelled paths, sub-minute frequencies, lists the code also changed since the last deploy, a change whose own report disagrees with `before`, and a construct that cannot be found or edited. Every edit is re-parsed and read back before a file is accepted, and a file that changed since the plan was made is not overwritten. Co-Authored-By: Claude Fable 5.1 --- .../write-back/__tests__/plan.spec.ts | 684 ++++++++++++++++++ packages/cli/src/services/write-back/plan.ts | 591 +++++++++++++++ 2 files changed, 1275 insertions(+) create mode 100644 packages/cli/src/services/write-back/__tests__/plan.spec.ts create mode 100644 packages/cli/src/services/write-back/plan.ts diff --git a/packages/cli/src/services/write-back/__tests__/plan.spec.ts b/packages/cli/src/services/write-back/__tests__/plan.spec.ts new file mode 100644 index 00000000..7aeede50 --- /dev/null +++ b/packages/cli/src/services/write-back/__tests__/plan.spec.ts @@ -0,0 +1,684 @@ +import fs from 'node:fs/promises' +import os from 'node:os' +import path from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { ApiCheck } from '../../../constructs/api-check.js' +import { CheckGroup } from '../../../constructs/check-group.js' +import { EmailAlertChannel } from '../../../constructs/email-alert-channel.js' +import { HeartbeatMonitor } from '../../../constructs/heartbeat-monitor.js' +import { Project } from '../../../constructs/project.js' +import { Session } from '../../../constructs/session.js' +import { TcpMonitor } from '../../../constructs/tcp-monitor.js' +import { UrlMonitor } from '../../../constructs/url-monitor.js' +import type { DiffEntry, DiffRedaction } from '../../../rest/projects.js' +import * as constructs from '../../../constructs/index.js' +import { AgenticCheck } from '../../../constructs/agentic-check.js' +import { Check, RuntimeCheck, RepairableRuntimeCheck } from '../../../constructs/check.js' +import { CheckGroupV1 } from '../../../constructs/check-group-v1.js' +import { GrpcMonitor } from '../../../constructs/grpc-monitor.js' +import { Monitor } from '../../../constructs/monitor.js' +import * as literalEdit from '../literal-edit.js' +import { applyWriteBack, type ConstructClass, planWriteBack, RULES_BY_CLASS } from '../plan.js' + +vi.mock('../literal-edit.js', async importOriginal => { + const original = await importOriginal() + return { ...original, evaluateLiteral: vi.fn(original.evaluateLiteral) } +}) + +/** + * The planner (`plan.ts`): which remote changes of a plan become edits of + * which file, what is refused and why, and that the files are written as + * planned. + */ + +let dir: string +let project: Project + +/** Writes a source file into the temp project and declares the constructs it holds while `Session` points at it. */ +async function declare (name: string, source: string, build: () => T): Promise { + const file = path.join(dir, name) + await fs.writeFile(file, source, 'utf8') + Session.checkFileAbsolutePath = file + try { + return build() + } finally { + Session.checkFileAbsolutePath = undefined + } +} + +const read = (name: string) => fs.readFile(path.join(dir, name), 'utf8') + +/** The rule table the API reports for a check: env var values and header values blanked when locked. */ +const CHECK_REDACTIONS: DiffRedaction[] = [ + { path: '/environmentVariables/*/value', kind: 'value', when: 'lockedOrSecret' }, + { path: '/request/headers/*/value', kind: 'value', when: 'locked' }, + { path: '/request/basicAuth/password', kind: 'value' }, +] + +const API_SOURCE = `import { ApiCheck, Frequency } from 'checkly/constructs' + +new ApiCheck('api', { + name: 'API', + activated: true, + tags: ['a'], + frequency: 10, + request: { + url: 'https://example.com', + method: 'GET', + }, +}) + +new ApiCheck('other', { + name: 'Other', + frequency: Frequency.EVERY_5M, + request: { url: 'https://example.com/other', method: 'GET' }, +}) +` + +function apiEntry (overrides: Partial = {}): DiffEntry { + return { + type: 'check', + logicalId: 'api', + physicalId: 'a1', + action: 'UPDATE', + changes: [{ path: '/name', origin: 'remote', before: 'API', after: 'API renamed' }], + before: { + id: 'a1', + checkType: 'API', + name: 'API renamed', + activated: true, + tags: ['a', 'b'], + frequency: 10, + request: { url: 'https://example.com', method: 'GET', headers: [], basicAuth: { username: '', password: '' } }, + environmentVariables: [], + }, + redactions: CHECK_REDACTIONS, + ...overrides, + } +} + +beforeEach(async () => { + dir = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), 'write-back-'))) + Session.reset() + Session.project = new Project('proj', { name: 'Project' }) + project = Session.project +}) + +afterEach(async () => { + Session.reset() + await fs.rm(dir, { recursive: true, force: true }) +}) + +describe('planWriteBack', () => { + it('writes a remote change into the construct and reports it', async () => { + await declare('api.check.ts', API_SOURCE, () => { + new ApiCheck('api', { name: 'API', request: { url: 'https://example.com', method: 'GET' } }) + new ApiCheck('other', { name: 'Other', request: { url: 'https://example.com/other', method: 'GET' } }) + }) + const plan = await planWriteBack({ diff: [apiEntry()], project, cwd: dir }) + expect(plan.skipped).toEqual([]) + expect(plan.applied).toEqual([{ + file: 'api.check.ts', + type: 'check', + logicalId: 'api', + property: 'name', + previous: '\'API\'', + rendered: '\'API renamed\'', + replacesLocalEdit: false, + }]) + expect(plan.files).toEqual([{ + path: path.join(dir, 'api.check.ts'), + text: API_SOURCE.replace('\'API\'', '\'API renamed\''), + original: API_SOURCE, + }]) + // Planning touches nothing. + expect(await read('api.check.ts')).toBe(API_SOURCE) + await applyWriteBack(plan) + expect(await read('api.check.ts')).toBe(API_SOURCE.replace('\'API\'', '\'API renamed\'')) + }) + + it('says nothing about a resource the deploy removes', async () => { + const plan = await planWriteBack({ + diff: [{ + type: 'check', + logicalId: 'gone', + action: 'DELETE', + changes: [{ path: '/name', origin: 'remote', before: 'a', after: 'b' }], + }], + project, + cwd: dir, + }) + expect(plan.skipped).toEqual([]) + }) + + it('ignores entries with no remote change and folded relations', async () => { + await declare('api.check.ts', API_SOURCE, () => { + new ApiCheck('api', { name: 'API', request: { url: 'https://example.com', method: 'GET' } }) + }) + const plan = await planWriteBack({ + diff: [ + apiEntry({ changes: [{ path: '/name', origin: 'code', before: 'API', after: 'API v2' }] }), + apiEntry({ foldedInto: { type: 'check', logicalId: 'api' } }), + { type: 'check', logicalId: 'api', action: 'UNCHANGED' }, + ], + project, + cwd: dir, + }) + expect(plan.applied).toEqual([]) + expect(plan.skipped).toEqual([]) + expect(plan.files).toEqual([]) + }) + + it('marks a change the code made too, and writes the account value', async () => { + await declare('api.check.ts', API_SOURCE, () => { + new ApiCheck('api', { name: 'API', request: { url: 'https://example.com', method: 'GET' } }) + }) + const plan = await planWriteBack({ + diff: [apiEntry({ + changes: [{ + path: '/name', origin: 'both', before: 'API', after: 'API local', remote: { before: 'API', after: 'API renamed' }, + }], + })], + project, + cwd: dir, + }) + expect(plan.applied).toMatchObject([{ property: 'name', rendered: '\'API renamed\'', replacesLocalEdit: true }]) + }) + + it('rewrites a whole set when elements were added and removed, and a plain list at its path', async () => { + await declare('api.check.ts', API_SOURCE, () => { + new ApiCheck('api', { name: 'API', request: { url: 'https://example.com', method: 'GET' } }) + }) + const plan = await planWriteBack({ + diff: [apiEntry({ + changes: [ + { path: '/tags/k1', origin: 'remote', after: 'b' }, + { path: '/tags/k2', origin: 'remote', before: 'c' }, + { path: '/request/headers', origin: 'remote', before: [], after: [{ key: 'x', value: '1', locked: false }] }, + ], + before: { ...apiEntry().before, request: { url: 'https://example.com', method: 'GET', headers: [{ key: 'x', value: '1', locked: false }] } }, + })], + project, + cwd: dir, + }) + expect(plan.skipped).toEqual([]) + expect(plan.applied.map(line => [line.property, line.rendered])).toEqual([ + ['tags', '[\'a\', \'b\']'], + ['request.headers', `[ + { + key: 'x', + value: '1', + locked: false, + }, + ]`], + ]) + }) + + it('refuses a list the code also changed, and one whose changes disagree with the account', async () => { + await declare('api.check.ts', API_SOURCE, () => { + new ApiCheck('api', { name: 'API', request: { url: 'https://example.com', method: 'GET' } }) + }) + const local = await planWriteBack({ + diff: [apiEntry({ + changes: [ + { path: '/tags/k1', origin: 'remote', after: 'b' }, + { path: '/tags/k2', origin: 'code', after: 'z' }, + ], + })], + project, + cwd: dir, + }) + expect(local.applied).toEqual([]) + expect(local.skipped).toEqual(['check api tags: your code also changed it since the last deploy; merge by hand']) + + const stale = await planWriteBack({ + diff: [ + apiEntry({ changes: [{ path: '/tags/k1', origin: 'remote', after: 'z' }] }), + apiEntry({ logicalId: 'api', changes: [{ path: '/tags/k2', origin: 'remote', before: 'a' }] }), + ], + project, + cwd: dir, + }) + expect(stale.applied).toEqual([]) + expect(stale.skipped).toEqual([ + 'check api tags: Checkly reported two different current values', + 'check api tags: Checkly reported two different current values', + ]) + + // One change carrying both sides is checked both ways. + const swapped = await planWriteBack({ + diff: [apiEntry({ changes: [{ path: '/tags/k1', origin: 'remote', before: 'a', after: 'b' }] })], + project, + cwd: dir, + }) + expect(swapped.skipped).toEqual(['check api tags: Checkly reported two different current values']) + + const noRemote = await planWriteBack({ + diff: [apiEntry({ changes: [{ path: '/name', origin: 'both', before: 'API', after: 'API local' }] })], + project, + cwd: dir, + }) + expect(noRemote.skipped).toEqual(['check api name: Checkly did not report the value it holds']) + }) + + it('refuses what the table, the redactions and the change reports rule out', async () => { + await declare('api.check.ts', API_SOURCE, () => { + new ApiCheck('api', { name: 'API', request: { url: 'https://example.com', method: 'GET' } }) + }) + const before = { + ...apiEntry().before, + description: 'x'.repeat(300), + frequency: 0, + frequencyOffset: 30, + groupId: 7, + retryStrategy: { type: 'FIXED' }, + script: 'console.log(1)', + environmentVariables: [{ key: 'TOKEN', value: 'shh', locked: true }], + request: { + url: 'https://example.com', + method: 'GET', + headers: [{ key: 'auth', value: 'shh', locked: true }], + basicAuth: { username: 'u', password: 'p' }, + }, + } + const plan = await planWriteBack({ + diff: [apiEntry({ + before, + changes: [ + { path: '/description', origin: 'remote', before: 'd', after: { $hash: 'abc' } }, + { path: '/frequency', origin: 'remote', before: 10, after: 0 }, + { path: '/frequencyOffset', origin: 'remote', after: 30 }, + { path: '/groupId', origin: 'remote', after: 7 }, + { path: '/retryStrategy', origin: 'remote', before: null, after: { type: 'FIXED' } }, + { path: '/script', origin: 'remote', before: 'a', after: 'console.log(1)' }, + { path: '/environmentVariables', origin: 'remote', before: [], after: [{ key: 'TOKEN', value: { $masked: 'changed' }, locked: true }], secret: true }, + { path: '/request/headers', origin: 'remote', before: [], after: [{ key: 'auth', value: '', locked: true }] }, + { path: '/request/basicAuth/password', origin: 'remote', before: '', after: 'p' }, + { path: '/codeBundle', origin: 'remote', cause: 'a new code bundle' }, + { path: '/alertChannels/k', origin: 'unmanaged', before: { alertChannelId: 1 } }, + { path: '/request/url', origin: 'remote', before: 'https://example.com', after: 'https://elsewhere.example.com' }, + { path: '/activated', origin: 'remote', before: true }, + ], + })], + project, + cwd: dir, + }) + // The long description came back in full in `before`, so it is the one thing written. + expect(plan.applied.map(line => line.property)).toEqual(['description']) + expect(plan.skipped).toEqual([ + 'check api /frequencyOffset: a sub-minute schedule; use Frequency.EVERY_*S', + 'check api /groupId: references another resource', + 'check api /retryStrategy: spelled by the CLI, not a construct property', + 'check api /script: not a property this tool can update', + 'check api /environmentVariables: a secret changed; Checkly does not return its value', + 'check api /codeBundle: a new code bundle: content, not a property', + 'check api frequency: a sub-minute schedule; use Frequency.EVERY_*S', + 'check api request.headers: contains a locked or secret value that Checkly does not return', + 'check api request.basicAuth: contains a locked or secret value that Checkly does not return', + 'check api request.url: Checkly reported two different current values', + 'check api activated: Checkly reported two different current values', + ]) + }) + + it('writes a long value from `before` when the change only carries its hash', async () => { + await declare('api.check.ts', API_SOURCE, () => { + new ApiCheck('api', { name: 'API', request: { url: 'https://example.com', method: 'GET' } }) + }) + const long = 'x'.repeat(300) + const plan = await planWriteBack({ + diff: [apiEntry({ + before: { ...apiEntry().before, description: long }, + changes: [{ path: '/description', origin: 'remote', before: 'd', after: { $hash: 'abc' } }], + })], + project, + cwd: dir, + }) + expect(plan.applied).toMatchObject([{ property: 'description', previous: undefined, rendered: `'${long}'` }]) + }) + + it('refuses a resource with no state, no redaction table, or a class this module does not know', async () => { + await declare('api.check.ts', API_SOURCE, () => { + new ApiCheck('api', { name: 'API', request: { url: 'https://example.com', method: 'GET' } }) + }) + class Wrapped extends ApiCheck {} + await declare('wrapped.check.ts', `import { ApiCheck } from 'checkly'\nnew Wrapped('wrapped', { name: 'w' })\n`, () => { + new Wrapped('wrapped', { name: 'w', request: { url: 'https://example.com', method: 'GET' } }) + }) + await declare('email.ts', `import { EmailAlertChannel } from 'checkly'\nnew EmailAlertChannel('mail', { address: 'a@b.c' })\n`, () => { + new EmailAlertChannel('mail', { address: 'a@b.c' }) + }) + const plan = await planWriteBack({ + diff: [ + apiEntry({ before: undefined, redactions: undefined }), + apiEntry({ redactions: undefined }), + apiEntry({ logicalId: 'wrapped' }), + apiEntry({ logicalId: 'gone' }), + apiEntry({ + type: 'alert-channel', + logicalId: 'mail', + changes: [{ path: '/config/address', origin: 'remote', before: 'a@b.c', after: 'x@b.c' }], + }), + ], + project, + cwd: dir, + }) + expect(plan.applied).toEqual([]) + expect(plan.skipped).toEqual([ + 'check api: Checkly did not report its current state', + 'check api: Checkly did not report which of its values are secret', + 'check wrapped: Wrapped is not a class from checkly/constructs', + 'check gone: not found in the project', + 'alert-channel mail: updating the code is supported for checks and check groups only', + ]) + }) + + it('finds groups and monitors through their exported aliases and mapped paths', async () => { + await declare('group.check.ts', `import { CheckGroup, HeartbeatCheck, UrlMonitor, TcpMonitor } from 'checkly/constructs' + +export const group = new CheckGroup('grp', { + name: 'Group', + concurrency: 2, + apiCheckDefaults: { url: 'https://example.com' }, +}) + +new HeartbeatCheck('beat', { name: 'Beat', period: 1, periodUnit: 'hours', grace: 5, graceUnit: 'minutes' }) + +new UrlMonitor('url', { name: 'Url', request: { url: 'https://example.com' } }) + +new TcpMonitor('tcp', { name: 'Tcp', request: { hostname: 'example.com', port: 443 } }) +`, () => { + new CheckGroup('grp', { name: 'Group', concurrency: 2, apiCheckDefaults: { url: 'https://example.com' } }) + new HeartbeatMonitor('beat', { name: 'Beat', period: 1, periodUnit: 'hours', grace: 5, graceUnit: 'minutes' }) + new UrlMonitor('url', { name: 'Url', request: { url: 'https://example.com' } }) + new TcpMonitor('tcp', { name: 'Tcp', request: { hostname: 'example.com', port: 443 } }) + }) + const plan = await planWriteBack({ + diff: [ + { + type: 'check-group', + logicalId: 'grp', + physicalId: 1, + action: 'UPDATE', + changes: [ + { path: '/concurrency', origin: 'remote', before: 2, after: 5 }, + { path: '/apiCheckDefaults/url', origin: 'remote', before: 'https://example.com', after: 'https://api.example.com' }, + { path: '/runParallel', origin: 'remote', before: false, after: true }, + ], + before: { id: 1, name: 'Group', concurrency: 5, apiCheckDefaults: { url: 'https://api.example.com' }, runParallel: true }, + redactions: [], + }, + { + type: 'check', + logicalId: 'beat', + action: 'UPDATE', + changes: [{ path: '/heartbeat/period', origin: 'remote', before: 1, after: 2 }], + before: { checkType: 'HEARTBEAT', name: 'Beat', heartbeat: { period: 2, periodUnit: 'hours', grace: 5, graceUnit: 'minutes' } }, + redactions: [], + }, + { + type: 'check', + logicalId: 'url', + action: 'UPDATE', + changes: [ + { path: '/request/url', origin: 'remote', before: 'https://example.com', after: 'https://www.example.com' }, + { path: '/maxResponseTime', origin: 'remote', before: 30000, after: 20000 }, + ], + before: { checkType: 'URL', name: 'Url', request: { url: 'https://www.example.com' }, maxResponseTime: 20000 }, + redactions: [], + }, + { + type: 'check', + logicalId: 'tcp', + action: 'UPDATE', + changes: [{ path: '/request/hostname', origin: 'remote', before: 'example.com', after: 'other.example.com' }], + before: { checkType: 'TCP', name: 'Tcp', request: { hostname: 'other.example.com', port: 443 } }, + redactions: [], + }, + ], + project, + cwd: dir, + }) + expect(plan.skipped).toEqual([ + 'check-group grp /runParallel: spelled by the CLI, not a construct property', + 'check tcp /request/hostname: not a property this tool can update', + ]) + expect(plan.applied.map(line => [line.logicalId, line.property, line.rendered])).toEqual([ + ['grp', 'concurrency', '5'], + ['grp', 'apiCheckDefaults.url', '\'https://api.example.com\''], + ['beat', 'period', '2'], + ['url', 'request.url', '\'https://www.example.com\''], + ['url', 'maxResponseTime', '20000'], + ]) + expect(plan.files).toHaveLength(1) + expect(plan.files[0].text).toBe(`import { CheckGroup, HeartbeatCheck, UrlMonitor, TcpMonitor } from 'checkly/constructs' + +export const group = new CheckGroup('grp', { + name: 'Group', + concurrency: 5, + apiCheckDefaults: { url: 'https://api.example.com' }, +}) + +new HeartbeatCheck('beat', { name: 'Beat', period: 2, periodUnit: 'hours', grace: 5, graceUnit: 'minutes' }) + +new UrlMonitor('url', { name: 'Url', request: { url: 'https://www.example.com' }, maxResponseTime: 20000 }) + +new TcpMonitor('tcp', { name: 'Tcp', request: { hostname: 'example.com', port: 443 } }) +`) + }) + + it('gives each class only the properties it takes', async () => { + await declare('mixed.check.ts', `import { AgenticCheck, GrpcMonitor } from 'checkly/constructs' +new AgenticCheck('agent', { name: 'Agent', prompt: 'p' }) +new GrpcMonitor('grpc', { name: 'Grpc', request: { host: 'example.com', port: 443, service: 'S', method: 'M' } }) +`, () => { + new AgenticCheck('agent', { name: 'Agent', prompt: 'p' } as any) + new GrpcMonitor('grpc', { name: 'Grpc', request: { host: 'example.com', port: 443, service: 'S', method: 'M' } } as any) + }) + const plan = await planWriteBack({ + diff: [ + { + type: 'check', + logicalId: 'agent', + action: 'UPDATE', + changes: [ + { path: '/shouldFail', origin: 'remote', before: false, after: true }, + { path: '/muted', origin: 'remote', before: false, after: true }, + ], + before: { checkType: 'AGENTIC', name: 'Agent', shouldFail: true, muted: true }, + redactions: [], + }, + { + type: 'check', + logicalId: 'grpc', + action: 'UPDATE', + changes: [{ path: '/maxResponseTime', origin: 'remote', before: 5000, after: 9000 }], + before: { checkType: 'GRPC', name: 'Grpc', maxResponseTime: 9000 }, + redactions: [], + }, + ], + project, + cwd: dir, + }) + expect(plan.skipped).toEqual(['check agent /shouldFail: not a property this tool can update']) + expect(plan.applied.map(line => [line.logicalId, line.property, line.rendered])).toEqual([ + ['agent', 'muted', 'true'], + ['grpc', 'maxResponseTime', '9000'], + ]) + }) + + it('writes a period and its unit together or not at all', async () => { + await declare('beat.check.ts', `import { HeartbeatMonitor } from 'checkly/constructs' +const unit = 'hours' +new HeartbeatMonitor('beat', { name: 'Beat', period: 1, periodUnit: unit, grace: 5, graceUnit: 'minutes' }) +`, () => { + new HeartbeatMonitor('beat', { name: 'Beat', period: 1, periodUnit: 'hours', grace: 5, graceUnit: 'minutes' }) + }) + const plan = await planWriteBack({ + diff: [{ + type: 'check', + logicalId: 'beat', + action: 'UPDATE', + changes: [ + { path: '/heartbeat/period', origin: 'remote', before: 1, after: 30 }, + { path: '/heartbeat/periodUnit', origin: 'remote', before: 'hours', after: 'minutes' }, + { path: '/heartbeat/grace', origin: 'remote', before: 5, after: 10 }, + ], + before: { checkType: 'HEARTBEAT', name: 'Beat', heartbeat: { period: 30, periodUnit: 'minutes', grace: 10, graceUnit: 'minutes' } }, + redactions: [], + }], + project, + cwd: dir, + }) + expect(plan.skipped).toEqual([ + 'check beat period: written together with periodUnit', + 'check beat periodUnit: periodUnit is the variable unit, not a plain literal', + ]) + expect(plan.applied.map(line => [line.property, line.rendered])).toEqual([['grace', '10']]) + }) + + it('lists every construct class checkly/constructs exports, or excludes it on purpose', () => { + // Abstract bases never appear in a project; every other check or group + // class must be named so a new one cannot fall back to a base's rules. + const abstract = new Set([Check, RuntimeCheck, RepairableRuntimeCheck, Monitor]) + const exported = Object.values(constructs).filter((value): value is ConstructClass => + typeof value === 'function' + && (value.prototype instanceof Check || value.prototype instanceof CheckGroupV1 || value === CheckGroupV1) + && !abstract.has(value as ConstructClass)) + expect(exported.length).toBeGreaterThan(10) + for (const cls of exported) { + expect(RULES_BY_CLASS.has(cls), `${cls.name} has no rules`).toBe(true) + } + }) + + it('refuses a value carrying a marker, and a property the code spells as a helper', async () => { + await declare('api.check.ts', API_SOURCE, () => { + new ApiCheck('api', { name: 'API', request: { url: 'https://example.com', method: 'GET' } }) + new ApiCheck('other', { name: 'Other', request: { url: 'https://example.com/other', method: 'GET' } }) + }) + const plan = await planWriteBack({ + diff: [ + apiEntry({ + changes: [{ path: '/request/headers', origin: 'remote', before: [], after: [{ key: 'a', value: { $masked: 'changed' } }] }], + before: { ...apiEntry().before, request: { url: 'https://example.com', method: 'GET', headers: [{ key: 'a', value: { $masked: 'changed' } }] } }, + }), + apiEntry({ + logicalId: 'other', + changes: [{ path: '/frequency', origin: 'remote', before: 5, after: 10 }], + before: { checkType: 'API', name: 'Other', frequency: 10 }, + }), + ], + project, + cwd: dir, + }) + expect(plan.applied).toEqual([]) + expect(plan.skipped).toEqual([ + 'check api request.headers: contains a value Checkly does not return in full', + 'check other frequency: frequency is Frequency.EVERY_5M, not a plain literal', + ]) + }) + + it('edits the constructs it can find in a file and skips the one it cannot', async () => { + await declare('api.check.ts', `import { ApiCheck } from 'checkly/constructs' +const opts = { name: 'Other' } +new ApiCheck('api', { name: 'API' }) +new ApiCheck('other', opts) +`, () => { + new ApiCheck('api', { name: 'API', request: { url: 'https://example.com', method: 'GET' } }) + new ApiCheck('other', { name: 'Other', request: { url: 'https://example.com', method: 'GET' } }) + }) + const plan = await planWriteBack({ + diff: [ + apiEntry(), + apiEntry({ logicalId: 'other', before: { checkType: 'API', name: 'Other renamed' }, changes: [{ path: '/name', origin: 'remote', before: 'Other', after: 'Other renamed' }] }), + ], + project, + cwd: dir, + }) + expect(plan.skipped).toEqual(['check other: api.check.ts: its options are not a plain object literal']) + expect(plan.applied.map(line => line.logicalId)).toEqual(['api']) + expect(plan.files[0].text).toContain('new ApiCheck(\'api\', { name: \'API renamed\' })') + }) + + it('discards a whole file when an edit does not read back, even after another construct was edited', async () => { + await declare('api.check.ts', API_SOURCE, () => { + new ApiCheck('api', { name: 'API', request: { url: 'https://example.com', method: 'GET' } }) + new ApiCheck('other', { name: 'Other', request: { url: 'https://example.com/other', method: 'GET' } }) + }) + // The first construct reads back; the second's read-back is made to lie. + vi.mocked(literalEdit.evaluateLiteral).mockImplementationOnce(node => literalEdit.evaluateLiteral(node)) + .mockImplementationOnce(() => 'something else') + const plan = await planWriteBack({ + diff: [ + apiEntry(), + apiEntry({ logicalId: 'other', before: { checkType: 'API', name: 'Other renamed' }, changes: [{ path: '/name', origin: 'remote', before: 'Other', after: 'Other renamed' }] }), + ], + project, + cwd: dir, + }) + expect(plan.files).toEqual([]) + expect(plan.applied).toEqual([]) + expect(plan.skipped).toEqual([ + 'check api: api.check.ts: the edited file did not read back as expected at name', + 'check other: api.check.ts: the edited file did not read back as expected at name', + ]) + }) + + it('reports a construct whose call it cannot find or edit, and leaves the file alone', async () => { + await declare('api.check.ts', `import { ApiCheck } from 'checkly'\nconst opts = { name: 'API' }\nnew ApiCheck('api', opts)\n`, () => { + new ApiCheck('api', { name: 'API', request: { url: 'https://example.com', method: 'GET' } }) + }) + const plan = await planWriteBack({ diff: [apiEntry()], project, cwd: dir }) + expect(plan.files).toEqual([]) + expect(plan.skipped).toEqual(['check api: api.check.ts: its options are not a plain object literal']) + }) + + it('reports a file it cannot read', async () => { + await declare('api.check.ts', API_SOURCE, () => { + new ApiCheck('api', { name: 'API', request: { url: 'https://example.com', method: 'GET' } }) + }) + await fs.rm(path.join(dir, 'api.check.ts')) + const plan = await planWriteBack({ diff: [apiEntry()], project, cwd: dir }) + expect(plan.skipped).toEqual([expect.stringMatching(/^check api: could not read api\.check\.ts: ENOENT/)]) + }) +}) + +describe('applyWriteBack', () => { + it('keeps the file mode and writes through a symlink to its target', async () => { + const real = path.join(dir, 'real.ts') + // Group-writable: a bit the usual umask would clear on a fresh file. + await fs.writeFile(real, 'old') + await fs.chmod(real, 0o664) + const link = path.join(dir, 'link.ts') + await fs.symlink(real, link) + await applyWriteBack({ files: [{ path: link, text: 'new', original: 'old' }], applied: [], skipped: [] }) + expect(await fs.readFile(real, 'utf8')).toBe('new') + expect((await fs.lstat(link)).isSymbolicLink()).toBe(true) + expect((await fs.stat(real)).mode & 0o777).toBe(0o664) + expect(await fs.readdir(dir)).toEqual(['link.ts', 'real.ts']) + }) + + it('refuses a file that changed since the plan was made', async () => { + const file = path.join(dir, 'a.ts') + await fs.writeFile(file, 'edited meanwhile', 'utf8') + await expect(applyWriteBack({ files: [{ path: file, text: 'new', original: 'old' }], applied: [], skipped: [] })) + .rejects.toThrow(/the file changed since the plan was made/) + expect(await fs.readFile(file, 'utf8')).toBe('edited meanwhile') + }) + + it('says no file changed when the first write fails', async () => { + const bad = path.join(dir, 'missing', 'bad.ts') + await expect(applyWriteBack({ files: [{ path: bad, text: 'x', original: '' }], applied: [], skipped: [] })) + .rejects.toThrow(/No file was changed\.$/) + }) + + it('names the files already written when a later one fails', async () => { + const good = path.join(dir, 'good.ts') + await fs.writeFile(good, 'old', 'utf8') + const bad = path.join(dir, 'missing', 'bad.ts') + await expect(applyWriteBack({ files: [{ path: good, text: 'new', original: 'old' }, { path: bad, text: 'x', original: '' }], applied: [], skipped: [] })) + .rejects.toThrow(new RegExp(`Could not write ${bad}: .*Already updated: ${good}\\.`)) + expect(await fs.readFile(good, 'utf8')).toBe('new') + expect(await fs.readdir(dir)).toEqual(['good.ts']) + }) +}) diff --git a/packages/cli/src/services/write-back/plan.ts b/packages/cli/src/services/write-back/plan.ts new file mode 100644 index 00000000..9664a51d --- /dev/null +++ b/packages/cli/src/services/write-back/plan.ts @@ -0,0 +1,591 @@ +import crypto from 'node:crypto' +import fs from 'node:fs/promises' +import path from 'node:path' +import { isDeepStrictEqual } from 'node:util' + +import * as constructs from '../../constructs/index.js' +import { AgenticCheck } from '../../constructs/agentic-check.js' +import { ApiCheck } from '../../constructs/api-check.js' +import { BrowserCheck } from '../../constructs/browser-check.js' +import { CheckGroupV1 } from '../../constructs/check-group-v1.js' +import { CheckGroupV2 } from '../../constructs/check-group-v2.js' +import type { Construct } from '../../constructs/construct.js' +import { DnsMonitor } from '../../constructs/dns-monitor.js' +import { GrpcMonitor } from '../../constructs/grpc-monitor.js' +import { HeartbeatMonitor } from '../../constructs/heartbeat-monitor.js' +import { IcmpMonitor } from '../../constructs/icmp-monitor.js' +import { MultiStepCheck } from '../../constructs/multi-step-check.js' +import { PlaywrightCheck } from '../../constructs/playwright-check.js' +import type { Project, ProjectData } from '../../constructs/project.js' +import { SslMonitor } from '../../constructs/ssl-monitor.js' +import { TcpMonitor } from '../../constructs/tcp-monitor.js' +import { TracerouteMonitor } from '../../constructs/traceroute-monitor.js' +import { UrlMonitor } from '../../constructs/url-monitor.js' +import type { DiffChange, DiffEntry } from '../../rest/projects.js' +import { blankRedacted, nodeAt, pointerSegments, UnshapeableError } from '../deploy-diff/import-shape.js' +import { isShapeChangePath } from '../deploy-diff/shape-changes.js' +import { applyLiteralEdits, evaluateLiteral, type LiteralEdit, resolvePath } from './literal-edit.js' +import { findConstructOptions, parseSource, WriteBackSkipped } from './source-file.js' + +/** + * Turns the remote changes of a deploy plan into edits of the construct + * source files, and applies them. + * + * A remote change is a property that moved in the Checkly account since the + * last deploy (`origin: 'remote'`, or `'both'` when the code moved too). The + * value written is the account's current one, read from the entry's `before` + * — the deployed resource in the import format, which a full-detail preview + * carries — at the path the change names. The change list decides *which* + * paths are written; `before` supplies the values, because a change can + * carry a hash where `before` carries the text, and a set element change + * names one element where the code holds the whole list. + * + * Only what can be written without guessing is written. A property has to + * be one this module knows the construct spells as a literal (the table + * below); a value Checkly withholds — a credential the redaction table + * blanks, a masked secret, a hash — is never written; and when the change's + * own report of the current value disagrees with `before`, neither is + * trusted. Everything refused is listed with its reason. + */ + +export interface WriteBackOptions { + diff: readonly DiffEntry[] + project: Project + /** The directory file names are shown relative to. */ + cwd: string +} + +export interface WriteBackLine { + /** The edited file, relative to `cwd`. */ + file: string + type: string + logicalId: string + /** The construct property, dotted. */ + property: string + /** The source text replaced, or undefined when the property was added. */ + previous?: string + /** The source text written. */ + rendered: string + /** Whether the code had moved too (`origin: 'both'`), so a local edit is being replaced. */ + replacesLocalEdit: boolean +} + +export interface WriteBackPlan { + /** Each file's new text, with the text it was planned from. */ + files: { path: string, text: string, original: string }[] + applied: WriteBackLine[] + skipped: string[] +} + +/** How an import-format path maps onto a construct property. */ +interface Rule { + /** Segments of the import-format pointer. */ + pointer: string[] + /** The construct property path the pointer maps to. */ + target: string[] + /** A list the API reports per element rather than whole; the code holds the whole list. */ + set?: boolean + /** Properties that only mean something together are written together or not at all. */ + group?: string +} + +const identity = (...segments: string[]): Rule => ({ pointer: segments, target: segments }) +const set = (segment: string): Rule => ({ pointer: [segment], target: [segment], set: true }) +const under = (parent: string, keys: string[]): Rule[] => keys.map(key => identity(parent, key)) + +const CHECK_RULES: Rule[] = [ + identity('name'), identity('description'), identity('activated'), identity('muted'), identity('shouldFail'), + set('tags'), set('locations'), identity('frequency'), +] +const RESPONSE_TIME_RULES: Rule[] = [identity('degradedResponseTime'), identity('maxResponseTime')] +// `assertions` is left out on purpose: the construct spells them with +// `AssertionBuilder`, and the wire form carries keys the type does not. +const API_REQUEST_KEYS = [ + 'url', 'method', 'ipFamily', 'followRedirects', 'skipSSL', 'body', 'bodyType', 'headers', 'queryParameters', 'basicAuth', +] +const URL_REQUEST_KEYS = ['url', 'ipFamily', 'followRedirects', 'skipSSL'] +const HEARTBEAT_KEYS = ['period', 'periodUnit', 'grace', 'graceUnit'] +const GROUP_RULES: Rule[] = [ + identity('name'), identity('activated'), identity('muted'), set('tags'), set('locations'), identity('concurrency'), + identity('environmentVariables'), + ...under('apiCheckDefaults', ['url', 'headers', 'queryParameters', 'basicAuth']), +] + +/** + * The properties this module writes, per construct class: the ones whose + * import-format spelling and construct spelling are both literals with the + * same shape. `frequency` is included because the construct takes a number, + * with the sub-minute case excluded below. Anything else — references, + * helper-built values such as retry strategies, scripts, the TCP/DNS/ICMP + * request whose keys differ between the two spellings — is left to the user. + * + * Keyed by the exact class, not by `instanceof`: a class this table does not + * name gets nothing rather than a base class's rules, so a construct that + * omits one of them (`AgenticCheck` takes neither `shouldFail` nor + * `frequency`) can never be handed it. The spec checks that every construct + * class `checkly/constructs` exports is listed here or excluded on purpose. + */ +/** A construct class, as a map key. */ +export type ConstructClass = abstract new (...args: any[]) => Construct + +export const RULES_BY_CLASS: ReadonlyMap = new Map([ + [ApiCheck, [...CHECK_RULES, identity('environmentVariables'), ...RESPONSE_TIME_RULES, ...under('request', API_REQUEST_KEYS)]], + [BrowserCheck, [...CHECK_RULES, identity('environmentVariables')]], + [MultiStepCheck, [...CHECK_RULES, identity('environmentVariables')]], + [PlaywrightCheck, [...CHECK_RULES, identity('environmentVariables')]], + [AgenticCheck, CHECK_RULES.filter(rule => rule.pointer[0] !== 'shouldFail' && rule.pointer[0] !== 'frequency')], + [UrlMonitor, [...CHECK_RULES, ...RESPONSE_TIME_RULES, ...under('request', URL_REQUEST_KEYS)]], + [TcpMonitor, [...CHECK_RULES, ...RESPONSE_TIME_RULES]], + [DnsMonitor, [...CHECK_RULES, ...RESPONSE_TIME_RULES]], + [GrpcMonitor, [...CHECK_RULES, ...RESPONSE_TIME_RULES]], + [SslMonitor, [...CHECK_RULES, ...RESPONSE_TIME_RULES]], + [TracerouteMonitor, [...CHECK_RULES, ...RESPONSE_TIME_RULES]], + [IcmpMonitor, [...CHECK_RULES, identity('degradedPacketLossThreshold'), identity('maxPacketLossThreshold')]], + [HeartbeatMonitor, [ + ...CHECK_RULES, + ...HEARTBEAT_KEYS.map(key => ({ pointer: ['heartbeat', key], target: [key], group: key.startsWith('period') ? 'period' : 'grace' })), + ]], + [CheckGroupV1, GROUP_RULES], + [CheckGroupV2, GROUP_RULES], +]) + +/** Paths that name another resource or a relation rather than a value of this one. */ +const REFERENCE_PREFIXES = ['alertChannels', 'privateLocations', 'alertChannelSubscriptions', 'privateLocationAssignments', 'groupId'] + +/** The names `checkly/constructs` exports for a construct's class; empty for a class of the user's own. */ +function exportedNamesOf (construct: Construct): Set { + const names = new Set() + for (const [name, value] of Object.entries(constructs)) { + if (value === construct.constructor) { + names.add(name) + } + } + return names +} + +/** Whether a reported value is one of the API's stand-ins (`{ $hash }`, `{ $masked }`, `{ $json }`, `{ $ref }`) or holds one. */ +function withheld (value: unknown): boolean { + if (Array.isArray(value)) { + return value.some(withheld) + } + if (value !== null && typeof value === 'object') { + const keys = Object.keys(value as object) + if (keys.some(key => key === '$hash' || key === '$masked' || key === '$json' || key === '$ref')) { + return true + } + return Object.values(value as object).some(withheld) + } + return false +} + +const startsWith = (segments: readonly string[], prefix: readonly string[]): boolean => + prefix.length <= segments.length && prefix.every((segment, i) => segments[i] === segment) + +/** + * The account-side value a change reports, if it reports one: `after` is the + * current value and `before` the one at the last deploy, read from `remote` + * when both sides moved. A missing key means the element is absent on that + * side. + */ +function reported (change: DiffChange, side: 'before' | 'after'): { present: boolean, value: unknown } { + const holder = change.origin === 'both' ? change.remote : change + return { present: holder !== undefined && side in holder, value: holder?.[side] } +} + +/** + * Whether the change's own report of the account value agrees with what + * `before` holds at the write path — the one guard on the assumption that + * `before` is the live resource. A set element is checked by membership: + * an element the change says is there must be in the list, one it says is + * gone must not be. A withheld value cannot be compared and passes. + */ +function agrees (change: DiffChange, rule: Rule, remaining: readonly string[], raw: unknown): boolean { + const current = reported(change, 'after') + if (rule.set && remaining.length > 0) { + if (!Array.isArray(raw)) { + return false + } + const previous = reported(change, 'before') + const holds = (value: unknown) => raw.some(element => isDeepStrictEqual(element, value)) + const added = !current.present || withheld(current.value) || holds(current.value) + const removed = !previous.present || withheld(previous.value) || !holds(previous.value) + return added && removed + } + // A path into a list that is not an index names an element by key, which + // only a set does; nothing can be checked against it. + if (remaining.length > 0 && Array.isArray(raw) && !/^\d+$/.test(remaining[0])) { + return false + } + if (!current.present) { + return nodeAt(raw, remaining) === undefined + } + return withheld(current.value) || isDeepStrictEqual(nodeAt(raw, remaining), current.value) +} + +interface Candidate { + rule: Rule + /** Every remote change that named this path; more than one for a set. */ + changes: DiffChange[] + /** Changes the code made under the same path since the last deploy, which `before` cannot hold. */ + local: DiffChange[] +} + +/** A reason a change cannot be written, or undefined when it may be. */ +function refusal (change: DiffChange, segments: readonly string[]): string | undefined { + if (change.cause !== undefined) { + return `${change.cause}: content, not a property` + } + if (change.secret === true) { + return 'a secret changed; Checkly does not return its value' + } + if (REFERENCE_PREFIXES.some(prefix => segments[0] === prefix)) { + return 'references another resource' + } + if (isShapeChangePath(change.path)) { + return 'spelled by the CLI, not a construct property' + } + if (segments[0] === 'frequencyOffset') { + return 'a sub-minute schedule; use Frequency.EVERY_*S' + } + if (segments[0] === 'intent') { + return 'the intent is ordered by the author; edit it by hand' + } + return undefined +} + +class EntryContext { + readonly label: string + + constructor (readonly entry: DiffEntry, readonly skipped: string[]) { + this.label = `${entry.type} ${entry.logicalId}` + } + + skip (reason: string, property?: string): void { + this.skipped.push(property === undefined ? `${this.label}: ${reason}` : `${this.label} ${property}: ${reason}`) + } +} + +/** The remote changes of an entry grouped by the construct path they write, or the reasons they cannot be. */ +function candidates (context: EntryContext, rules: readonly Rule[]): Candidate[] { + const byPath = new Map() + const segmentsOf = (change: DiffChange): string[] | undefined => { + try { + return pointerSegments(change.path) + } catch { + return undefined + } + } + const ruleFor = (segments: readonly string[]): Rule | undefined => + rules.find(rule => startsWith(segments, rule.pointer)) + for (const change of context.entry.changes ?? []) { + if (change.origin !== 'remote' && change.origin !== 'both') { + continue + } + const segments = segmentsOf(change) + if (segments === undefined) { + context.skip('not a property path', change.path) + continue + } + const reason = refusal(change, segments) + if (reason !== undefined) { + context.skip(reason, change.path) + continue + } + const rule = ruleFor(segments) + if (rule === undefined) { + context.skip('not a property this tool can update', change.path) + continue + } + const key = rule.target.join('.') + const candidate = byPath.get(key) ?? { rule, changes: [], local: [] } + candidate.changes.push(change) + byPath.set(key, candidate) + } + // A list is written whole from `before`, which knows nothing of an element + // the code added and has not deployed; such an edit must not be erased. + for (const change of context.entry.changes ?? []) { + if (change.origin !== 'code') { + continue + } + const segments = segmentsOf(change) + const rule = segments === undefined ? undefined : ruleFor(segments) + const candidate = rule === undefined ? undefined : byPath.get(rule.target.join('.')) + if (candidate !== undefined) { + candidate.local.push(change) + } + } + return [...byPath.values()] +} + +/** + * The value `before` holds at a candidate's path, or the reason it cannot be + * written: the redaction table blanked something under it, the API withheld + * it, or a change disagrees with it. + */ +function valueFor ( + context: EntryContext, + candidate: Candidate, + before: unknown, + blanked: unknown, +): { value: unknown } | undefined { + const { rule } = candidate + const segments = rule.pointer + const property = rule.target.join('.') + if (candidate.local.length > 0) { + context.skip('your code also changed it since the last deploy; merge by hand', property) + return undefined + } + const raw = nodeAt(before, segments) + if (!isDeepStrictEqual(raw, nodeAt(blanked, segments))) { + context.skip('contains a locked or secret value that Checkly does not return', property) + return undefined + } + if (withheld(raw)) { + context.skip('contains a value Checkly does not return in full', property) + return undefined + } + for (const change of candidate.changes) { + if (change.origin === 'both' && change.remote === undefined) { + context.skip('Checkly did not report the value it holds', property) + return undefined + } + const remaining = pointerSegments(change.path).slice(segments.length) + if (!agrees(change, rule, remaining, raw)) { + context.skip('Checkly reported two different current values', property) + return undefined + } + } + if (property === 'frequency') { + // The construct takes whole minutes as a number; anything else — zero + // with an offset, or the object spelling — is a helper's job. + const offsetReported = (context.entry.changes ?? []).some(change => change.path === '/frequencyOffset') + if (typeof raw !== 'number' || !Number.isInteger(raw) || raw <= 0 || offsetReported) { + context.skip('a sub-minute schedule; use Frequency.EVERY_*S', property) + return undefined + } + } + return { value: raw } +} + +interface FileWork { + /** The names `checkly/constructs` exports for the construct's class. */ + names: ReadonlySet + context: EntryContext + edits: (LiteralEdit & { replacesLocalEdit: boolean, group?: string })[] +} + +export async function planWriteBack ({ diff, project, cwd }: WriteBackOptions): Promise { + const skipped: string[] = [] + const byFile = new Map() + + for (const entry of diff) { + if (entry.foldedInto !== undefined || !(entry.changes ?? []).some(c => c.origin === 'remote' || c.origin === 'both')) { + continue + } + const context = new EntryContext(entry, skipped) + const construct: Construct | undefined = project.data[entry.type as keyof ProjectData]?.[entry.logicalId] + if (construct === undefined) { + // A resource the deploy removes has no construct to edit; only one the + // code still declares is worth a line. + if (String(entry.action).toUpperCase() === 'UPDATE') { + context.skip('not found in the project') + } + continue + } + const rules = RULES_BY_CLASS.get(construct.constructor as ConstructClass) + if (rules === undefined) { + context.skip(exportedNamesOf(construct).size === 0 + ? `${construct.constructor.name} is not a class from checkly/constructs` + : 'updating the code is supported for checks and check groups only') + continue + } + if (construct.checkFileAbsolutePath === undefined) { + context.skip('the file that declares it is not known') + continue + } + if (entry.before === undefined) { + context.skip('Checkly did not report its current state') + continue + } + let blanked: unknown + try { + blanked = blankRedacted(entry.before, entry.redactions) + } catch (err) { + if (err instanceof UnshapeableError) { + context.skip('Checkly did not report which of its values are secret') + continue + } + throw err + } + const edits: FileWork['edits'] = [] + for (const candidate of candidates(context, rules)) { + const found = valueFor(context, candidate, entry.before, blanked) + if (found !== undefined) { + edits.push({ + path: candidate.rule.target, + value: found.value, + replacesLocalEdit: candidate.changes.some(change => change.origin === 'both'), + group: candidate.rule.group, + }) + } + } + if (edits.length === 0) { + continue + } + const work = byFile.get(construct.checkFileAbsolutePath) ?? [] + work.push({ names: exportedNamesOf(construct), context, edits }) + byFile.set(construct.checkFileAbsolutePath, work) + } + + const files: WriteBackPlan['files'] = [] + const applied: WriteBackLine[] = [] + for (const [filePath, work] of byFile) { + const file = path.relative(cwd, filePath) + let text: string + try { + text = await fs.readFile(filePath, 'utf8') + } catch (err: any) { + for (const { context } of work) { + context.skip(`could not read ${file}: ${err.message}`) + } + continue + } + const original = text + const lines: WriteBackLine[] = [] + // Constructs of one file are edited one after another, each against a + // fresh parse of the text the previous one produced, so no range is stale. + for (const { names, context, edits } of work) { + const logicalId = context.entry.logicalId + let source + let options + try { + source = parseSource(filePath, text) + options = findConstructOptions(source, logicalId, names) + } catch (err) { + if (!(err instanceof WriteBackSkipped)) { + throw err + } + context.skip(`${file}: ${err.message}`) + continue + } + try { + // A member of a group the splicer refuses takes the rest of its group + // with it: a period without its unit would mean something else. + let attempt = edits + let result = applyLiteralEdits(source, options, attempt) + for (;;) { + const refused = new Set(result.skipped.map(skip => attempt.find(edit => edit.path === skip.path)?.group) + .filter((group): group is string => group !== undefined)) + const dropped = attempt.filter(edit => edit.group !== undefined && refused.has(edit.group) + && !result.skipped.some(skip => skip.path === edit.path)) + if (dropped.length === 0) { + break + } + for (const edit of dropped) { + context.skip(`written together with ${attempt.filter(e => e.group === edit.group && e !== edit).map(e => e.path.join('.')).join(', ')}`, edit.path.join('.')) + } + attempt = attempt.filter(edit => !dropped.includes(edit)) + result = applyLiteralEdits(source, options, attempt) + } + for (const skip of result.skipped) { + context.skip(skip.reason, skip.path.join('.')) + } + if (result.applied.length === 0) { + continue + } + // What was written must read back as what was meant, or the file is + // left alone: a splice that produced something else is a bug, and the + // user's source is not the place to find out. + const reparsed = findConstructOptions(parseSource(filePath, result.text), logicalId, names) + for (const edit of result.applied) { + const resolution = resolvePath(reparsed, edit.path) + if (resolution.kind !== 'found' || !isDeepStrictEqual(evaluateLiteral(resolution.node), edit.value)) { + throw new WriteBackSkipped(`the edited file did not read back as expected at ${edit.path.join('.')}`) + } + } + text = result.text + for (const edit of result.applied) { + lines.push({ + file, + type: context.entry.type, + logicalId, + property: edit.path.join('.'), + previous: edit.previous, + rendered: edit.rendered, + replacesLocalEdit: edits.find(e => e.path === edit.path)?.replacesLocalEdit ?? false, + }) + } + } catch (err) { + if (!(err instanceof WriteBackSkipped)) { + throw err + } + // A file that cannot be trusted after one construct's edits is not + // written at all, whatever the others produced. + for (const { context: other } of work) { + other.skip(`${file}: ${err.message}`) + } + text = original + lines.length = 0 + break + } + } + if (text !== original) { + files.push({ path: filePath, text, original }) + applied.push(...lines) + } + } + return { files, applied, skipped } +} + +/** + * Writes every planned file through a temporary file in the same directory, + * flushed to disk and then renamed over the target, so an interrupted write + * leaves the old file or the new one rather than a torn one. The target is + * the file itself, not a symlink to it, and it keeps its mode. + * + * @throws Error naming the files already rewritten when a later one fails. + */ +export async function applyWriteBack (plan: WriteBackPlan): Promise { + const written: string[] = [] + for (const file of plan.files) { + let temporary: string | undefined + try { + const target = await fs.realpath(file.path) + // The plan was made from what the file held then; a file edited since + // is not overwritten with a rewrite of its older self. + if (await fs.readFile(target, 'utf8') !== file.original) { + throw new Error('the file changed since the plan was made') + } + const { mode } = await fs.stat(target) + temporary = `${target}.${crypto.randomBytes(4).toString('hex')}.tmp` + const handle = await fs.open(temporary, 'wx') + let failed = false + try { + // Set after opening, since the mode passed to open is filtered by + // the umask; a filesystem without modes keeps its own. + await handle.chmod(mode).catch(() => undefined) + await handle.writeFile(file.text, 'utf8') + await handle.sync() + } catch (err) { + failed = true + throw err + } finally { + // On the way out with an error, the error is the one to report; on + // success, a close that fails is the write failing late. + await handle.close().catch(err => { + if (!failed) { + throw err + } + }) + } + await fs.rename(temporary, target) + written.push(file.path) + } catch (err: any) { + if (temporary !== undefined) { + await fs.rm(temporary, { force: true }).catch(() => undefined) + } + const done = written.length === 0 ? 'No file was changed.' : `Already updated: ${written.join(', ')}.` + throw new Error(`Could not write ${file.path}: ${err.message} ${done}`, { cause: err }) + } + } +} From a5cd62e177cd466a547833beb53ae0edaca51eca Mon Sep 17 00:00:00 2001 From: Simo Kinnunen Date: Tue, 22 Sep 2026 02:18:14 +0900 Subject: [PATCH 3/4] feat(deploy): offer to write remote changes into the code before asking to apply [RED-985] When the plan shows a resource edited in Checkly and the code can take the edit, the interactive prompt becomes a list: apply the changes, update the code with the values from Checkly and deploy nothing, or cancel. The cursor starts on Cancel, so Enter alone applies nothing, as the yes/no question's default did. Choosing the update lists what could not be written and why, rewrites the files, reports each property with its old and new value once the files hold it, and ends the run so the user can review the diff and deploy again. A failed write is reported as an error naming the files already rewritten. `--force`, `--dry-run`, read-only commands and the agent/CI envelope are unchanged; a command that supplies its own confirmation keeps it. Co-Authored-By: Claude Fable 5.1 --- .../src/ai-context/references/configure.md | 1 + .../__tests__/confirm-flow-deploy.spec.ts | 194 ++++++++++++++++++ .../__tests__/confirm-or-abort.spec.ts | 89 ++++++++ packages/cli/src/commands/authCommand.ts | 49 ++++- packages/cli/src/commands/deploy.ts | 64 +++++- packages/cli/src/helpers/command-preview.ts | 12 ++ packages/cli/src/services/write-back/plan.ts | 20 ++ 7 files changed, 417 insertions(+), 12 deletions(-) diff --git a/packages/cli/src/ai-context/references/configure.md b/packages/cli/src/ai-context/references/configure.md index 2e31e94f..f4626600 100644 --- a/packages/cli/src/ai-context/references/configure.md +++ b/packages/cli/src/ai-context/references/configure.md @@ -76,6 +76,7 @@ Run `npx checkly skills manage plan` for the full reference. - Deploy checks using the `npx checkly deploy` command. Use `--output` to see the created, updated, and deleted resources. Use `--verbose` to also include each resource's name and physical ID (UUID), which is useful for programmatically referencing deployed resources (e.g. `npx checkly checks get `). - Use `--preview` to see which resources a deploy would create, update, delete or keep, without applying it: an overview of every resource the deploy touches and a diff of each updated resource's construct as deployed against as in code. A plain interactive `checkly deploy` prints that same preview before asking the user to apply the changes or cancel. The machine-readable forms (`--dry-run`, and the `confirmation_required` envelope) additionally carry the individual properties that would change. +- When the preview shows a resource that was edited outside the project (in the web app or through the API), an interactive `checkly deploy` offers a third choice next to apply and cancel: update the code with the values from Checkly and deploy nothing. It rewrites only literal values (strings, numbers, booleans, and arrays or objects of those) inside the `new ApiCheck('id', { … })` call of checks and check groups, leaves the rest of the file untouched, and lists everything it could not update with the reason (helper values such as `Frequency.EVERY_5M`, references to other resources, secrets, scripts). It exists only in a terminal; there is no flag for it, and the `confirmation_required` envelope is unchanged. - Use `--skip-plan` to deploy without asking Checkly for a plan: nothing is previewed and no plan token is used, so the deploy applies whatever the account looks like when it runs. Resources to delete are still listed before the confirmation, but the code bundle is uploaded before it rather than after. Incompatible with `--preview`, `--dry-run`, `--plan-token` and `--prune-relations`. Prefer a planned deploy unless the plan itself is the problem. - Use `--prune-relations` to also delete the alert channel subscriptions and private location assignments on this project's checks and groups that the project does not manage. Without it they are only reported. diff --git a/packages/cli/src/commands/__tests__/confirm-flow-deploy.spec.ts b/packages/cli/src/commands/__tests__/confirm-flow-deploy.spec.ts index 285f8e2e..31f642d2 100644 --- a/packages/cli/src/commands/__tests__/confirm-flow-deploy.spec.ts +++ b/packages/cli/src/commands/__tests__/confirm-flow-deploy.spec.ts @@ -1,3 +1,5 @@ +import fs from 'node:fs/promises' +import os from 'node:os' import path from 'node:path' import { Parser } from '@oclif/core' @@ -63,6 +65,11 @@ vi.mock('prompts', () => ({ default: vi.fn(() => Promise.resolve({ confirm: true })), })) +vi.mock('../../services/write-back/plan', async importOriginal => { + const original = await importOriginal() + return { ...original, applyWriteBack: vi.fn(original.applyWriteBack) } +}) + import prompts from 'prompts' import { detectCliMode } from '../../helpers/cli-mode.js' @@ -77,6 +84,8 @@ import { import { Ok } from '../../services/check-parser/package-files/result.js' import { parseProject } from '../../services/project-parser.js' import { getGitRepoRoot } from '../../services/util.js' +import { applyWriteBack } from '../../services/write-back/plan.js' +import { ApiCheck } from '../../constructs/api-check.js' import { EmailAlertChannel } from '../../constructs/email-alert-channel.js' import { Project } from '../../constructs/project.js' import { Session } from '../../constructs/session.js' @@ -1061,3 +1070,188 @@ describe('deploy confirmCommand', () => { } }) }) + +describe('deploy write-back from a terminal', () => { + const SOURCE = `import { ApiCheck } from 'checkly/constructs' + +new ApiCheck('api', { + name: 'API', + request: { url: 'https://example.com', method: 'GET' }, +}) +` + const remoteEdit: DiffEntry = { + type: 'check', + logicalId: 'api', + physicalId: 'a1', + action: 'UPDATE', + changes: [ + { path: '/name', origin: 'remote', before: 'API', after: 'API renamed' }, + { path: '/frequency', origin: 'remote', before: 10, after: 5 }, + ], + before: { id: 'a1', checkType: 'API', name: 'API renamed', frequency: 5, request: { url: 'https://example.com', method: 'GET' } }, + redactions: [], + } + let dir: string + + /** The project with an API check declared in a real file, so the write-back has something to edit. */ + async function declareProjectWithFile () { + Session.reset() + Session.workspace = Ok({} as any) + const project = new Project('my-project', { name: 'My Project' }) + Session.project = project + const file = path.join(dir, 'api.check.ts') + await fs.writeFile(file, SOURCE, 'utf8') + Session.checkFileAbsolutePath = file + new ApiCheck('api', { name: 'API', request: { url: 'https://example.com', method: 'GET' } }) + Session.checkFileAbsolutePath = undefined + vi.mocked(parseProject).mockResolvedValue(project) + } + + beforeEach(async () => { + vi.clearAllMocks() + vi.mocked(detectCliMode).mockReturnValue('interactive') + storeBundle.mockResolvedValue({ key: 'stored-bundle-key' }) + vi.mocked(api.projects.deploy).mockResolvedValue({ data: { project: {} as any, diff: [] } }) + dir = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), 'deploy-write-back-'))) + await declareProjectWithFile() + }) + + afterEach(async () => { + Session.reset() + await fs.rm(dir, { recursive: true, force: true }) + }) + + it('offers to update the code when a resource was edited in Checkly, and does so instead of deploying', async () => { + planResolves([remoteEdit]) + vi.mocked(prompts).mockResolvedValue({ action: 'alternative:0' }) + const spy = vi.spyOn(process, 'cwd').mockReturnValue(dir) + const ctx = createCommandContext() + + try { + await expect(Deploy.prototype.run.call(ctx as any)).rejects.toThrow('EXIT_0') + } finally { + spy.mockRestore() + } + + expect(vi.mocked(prompts).mock.calls[0][0]).toMatchObject({ + type: 'select', + choices: [ + { value: 'apply' }, + { title: 'Update my code with the changes made in Checkly (deploys nothing)', value: 'alternative:0' }, + { value: 'cancel' }, + ], + }) + expect(await fs.readFile(path.join(dir, 'api.check.ts'), 'utf8')).toBe(`import { ApiCheck } from 'checkly/constructs' + +new ApiCheck('api', { + name: 'API renamed', + request: { url: 'https://example.com', method: 'GET' }, + frequency: 5, +}) +`) + const printed = ctx.logged.join('\n') + expect(printed).toContain('Updated 1 file:\n api.check.ts: check api name: \'API\' -> \'API renamed\'') + // A property the code does not set is added; a whole-minute frequency is + // a plain number the construct accepts. + expect(printed).toContain(' api.check.ts: check api frequency: not set -> 5') + expect(printed).toContain('Nothing was deployed. Review the changes, then run `checkly deploy` again.') + expect(storeBundle).not.toHaveBeenCalled() + expect(api.projects.deploy).not.toHaveBeenCalled() + }) + + it('does not offer the choice when no remote change could be written', async () => { + planResolves([{ + ...remoteEdit, + changes: [{ path: '/retryStrategy', origin: 'remote', before: null, after: { type: 'FIXED' } }], + }]) + vi.mocked(prompts).mockResolvedValue({ confirm: false }) + const ctx = createCommandContext() + + await expect(Deploy.prototype.run.call(ctx as any)).rejects.toThrow('EXIT_0') + + expect(vi.mocked(prompts).mock.calls[0][0]).toMatchObject({ type: 'confirm' }) + expect(await fs.readFile(path.join(dir, 'api.check.ts'), 'utf8')).toBe(SOURCE) + expect(api.projects.deploy).not.toHaveBeenCalled() + }) + + it('does not offer the choice for a resource whose class it cannot update, or for a secret', async () => { + planResolves([ + { + ...CHANGED, + redactions: [], + changes: [{ path: '/config/address', origin: 'remote', before: 'ops@example.com', after: 'new@example.com' }], + }, + { + ...remoteEdit, + changes: [{ path: '/environmentVariables', origin: 'remote', secret: true, after: [{ key: 'K', value: { $masked: 'changed' } }] }], + }, + ]) + vi.mocked(prompts).mockResolvedValue({ confirm: false }) + const ctx = createCommandContext() + + await expect(Deploy.prototype.run.call(ctx as any)).rejects.toThrow('EXIT_0') + + expect(vi.mocked(prompts).mock.calls[0][0]).toMatchObject({ type: 'confirm' }) + }) + + it('lists what it could not update and changes nothing when nothing applies after all', async () => { + // A writable path that turns out unwritable only once the file is read. + await fs.writeFile(path.join(dir, 'api.check.ts'), SOURCE.replace('name: \'API\'', 'name: title'), 'utf8') + planResolves([{ ...remoteEdit, changes: [remoteEdit.changes![0]] }]) + vi.mocked(prompts).mockResolvedValue({ action: 'alternative:0' }) + const ctx = createCommandContext() + + await expect(Deploy.prototype.run.call(ctx as any)).rejects.toThrow('EXIT_0') + + const printed = ctx.logged.join('\n') + expect(printed).toContain('Not updated (edit these by hand):\n check api name: name is the variable title, not a plain literal') + expect(printed).toContain('Nothing in the code could be updated automatically, so nothing was changed.') + expect(applyWriteBack).not.toHaveBeenCalled() + expect(api.projects.deploy).not.toHaveBeenCalled() + }) + + it('reports a write that fails as an error, after listing nothing as updated', async () => { + planResolves([remoteEdit]) + vi.mocked(prompts).mockResolvedValue({ action: 'alternative:0' }) + vi.mocked(applyWriteBack).mockRejectedValueOnce(new Error('Could not write api.check.ts: EACCES. No file was changed.')) + const ctx = createCommandContext() + + await expect(Deploy.prototype.run.call(ctx as any)).rejects.toThrow('EXIT_1') + + expect(ctx.style.longError).toHaveBeenCalledWith('Could not update your code.', 'Could not write api.check.ts: EACCES. No file was changed.') + expect(ctx.logged.join('\n')).not.toContain('Updated') + expect(api.projects.deploy).not.toHaveBeenCalled() + }) + + it('asks the plain yes/no question when nothing was edited in Checkly', async () => { + planResolves([{ ...CHANGED, redactions: [] }]) + vi.mocked(prompts).mockResolvedValue({ confirm: true }) + const ctx = createCommandContext() + + await Deploy.prototype.run.call(ctx as any) + + expect(vi.mocked(prompts).mock.calls[0][0]).toMatchObject({ type: 'confirm', message: 'Apply these changes?' }) + expect(api.projects.deploy).toHaveBeenCalledOnce() + }) + + it('applies the plan when the user chooses to', async () => { + planResolves([remoteEdit]) + vi.mocked(prompts).mockResolvedValue({ action: 'apply' }) + const ctx = createCommandContext() + + await Deploy.prototype.run.call(ctx as any) + + expect(await fs.readFile(path.join(dir, 'api.check.ts'), 'utf8')).toBe(SOURCE) + expect(api.projects.deploy).toHaveBeenCalledOnce() + }) + + it('never prompts a forced run', async () => { + planResolves([remoteEdit]) + const ctx = createCommandContext({ force: true }) + + await Deploy.prototype.run.call(ctx as any) + + expect(prompts).not.toHaveBeenCalled() + expect(api.projects.deploy).toHaveBeenCalledOnce() + }) +}) diff --git a/packages/cli/src/commands/__tests__/confirm-or-abort.spec.ts b/packages/cli/src/commands/__tests__/confirm-or-abort.spec.ts index cbe64ac2..a754425f 100644 --- a/packages/cli/src/commands/__tests__/confirm-or-abort.spec.ts +++ b/packages/cli/src/commands/__tests__/confirm-or-abort.spec.ts @@ -285,3 +285,92 @@ describe('confirmOrAbort', () => { expect(output.status).toBe('confirmation_required') }) }) + +describe('confirmOrAbort with alternatives', () => { + const alternative = { title: 'Do the other thing', run: vi.fn(() => Promise.resolve()) } + const withAlternative: CommandPreview = { + ...basePreview, + question: 'Apply these changes?', + terminal: { plan: () => 'the plan', changes: ['deploy'], alternatives: [alternative] }, + } + + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(detectCliMode).mockReturnValue('interactive') + }) + + it('asks a list with the alternative between apply and cancel, starting on cancel', async () => { + vi.mocked(prompts).mockResolvedValue({ action: 'apply' }) + const ctx = createMockCommand() + + await AuthCommand.prototype.confirmOrAbort.call(ctx as any, withAlternative, { force: false }) + + expect(vi.mocked(prompts).mock.calls[0][0]).toEqual({ + name: 'action', + type: 'select', + message: 'Apply these changes?', + choices: [ + { title: 'Yes, apply these changes', value: 'apply' }, + { title: 'Do the other thing', value: 'alternative:0' }, + { title: 'Cancel', value: 'cancel' }, + ], + initial: 2, + }) + expect(alternative.run).not.toHaveBeenCalled() + expect(ctx.exit).not.toHaveBeenCalled() + }) + + it('runs the chosen alternative and then ends the command without applying', async () => { + vi.mocked(prompts).mockResolvedValue({ action: 'alternative:0' }) + const ctx = createMockCommand() + + await expect(AuthCommand.prototype.confirmOrAbort.call(ctx as any, withAlternative, { force: false })) + .rejects.toThrow('EXIT_0') + + expect(alternative.run).toHaveBeenCalledOnce() + }) + + it('ends the command on cancel and on an aborted prompt', async () => { + for (const answer of [{ action: 'cancel' }, {}]) { + vi.mocked(prompts).mockResolvedValue(answer) + const ctx = createMockCommand() + await expect(AuthCommand.prototype.confirmOrAbort.call(ctx as any, withAlternative, { force: false })) + .rejects.toThrow('EXIT_0') + } + expect(alternative.run).not.toHaveBeenCalled() + }) + + it('keeps the yes/no question when there is nothing else to offer', async () => { + vi.mocked(prompts).mockResolvedValue({ confirm: true }) + const ctx = createMockCommand() + const plain: CommandPreview = { ...withAlternative, terminal: { plan: () => 'the plan', changes: ['deploy'], alternatives: [] } } + + await AuthCommand.prototype.confirmOrAbort.call(ctx as any, plain, { force: false }) + + expect(vi.mocked(prompts).mock.calls[0][0]).toMatchObject({ type: 'confirm', message: 'Apply these changes?' }) + }) + + it('lets a command-supplied confirm win over the alternatives', async () => { + const ctx = createMockCommand() + const interactiveConfirm = vi.fn(() => Promise.resolve(true)) + + await AuthCommand.prototype.confirmOrAbort.call(ctx as any, withAlternative, { force: false, interactiveConfirm }) + + expect(interactiveConfirm).toHaveBeenCalledOnce() + expect(prompts).not.toHaveBeenCalled() + }) + + it('never offers the alternative to an agent, a forced run or a dry run', async () => { + vi.mocked(detectCliMode).mockReturnValue('agent') + let ctx = createMockCommand() + await expect(AuthCommand.prototype.confirmOrAbort.call(ctx as any, withAlternative, { force: false })).rejects.toThrow('EXIT_2') + expect(JSON.parse(ctx.logged[0])).not.toHaveProperty('terminal') + + ctx = createMockCommand() + await AuthCommand.prototype.confirmOrAbort.call(ctx as any, withAlternative, { force: true }) + ctx = createMockCommand() + await expect(AuthCommand.prototype.confirmOrAbort.call(ctx as any, withAlternative, { force: false, dryRun: true })).rejects.toThrow('EXIT_0') + expect(prompts).not.toHaveBeenCalled() + expect(alternative.run).not.toHaveBeenCalled() + }) +}) diff --git a/packages/cli/src/commands/authCommand.ts b/packages/cli/src/commands/authCommand.ts index c2d09073..0407f628 100644 --- a/packages/cli/src/commands/authCommand.ts +++ b/packages/cli/src/commands/authCommand.ts @@ -85,18 +85,45 @@ export abstract class AuthCommand extends BaseCommand { this.log(formatPreviewForTerminal(preview)) this.log() - const confirmed = options.interactiveConfirm - ? await options.interactiveConfirm() - : (await prompts({ - name: 'confirm', - type: 'confirm', - message: preview.question ?? 'Proceed?', - })).confirm - - if (!confirmed) { - return this.exit(0) + if (options.interactiveConfirm !== undefined) { + if (!await options.interactiveConfirm()) { + return this.exit(0) + } + return } - return + + const question = preview.question ?? 'Proceed?' + const alternatives = preview.terminal?.alternatives ?? [] + if (alternatives.length === 0) { + const { confirm } = await prompts({ name: 'confirm', type: 'confirm', message: question }) + if (!confirm) { + return this.exit(0) + } + return + } + + // With something else on offer, the yes/no question becomes a list. The + // cursor starts on Cancel, so Enter alone applies nothing — as the + // confirm's default of No did. + const choices = [ + { title: 'Yes, apply these changes', value: 'apply' }, + ...alternatives.map((alternative, index) => ({ title: alternative.title, value: `alternative:${index}` })), + { title: 'Cancel', value: 'cancel' }, + ] + const { action } = await prompts({ + name: 'action', + type: 'select', + message: question, + choices, + initial: choices.length - 1, + }) + if (action === 'apply') { + return + } + if (typeof action === 'string' && action.startsWith('alternative:')) { + await alternatives[Number(action.slice('alternative:'.length))].run() + } + return this.exit(0) } // Agent or CI mode: output structured JSON and exit 2 diff --git a/packages/cli/src/commands/deploy.ts b/packages/cli/src/commands/deploy.ts index ed61ebb0..8e4931cc 100644 --- a/packages/cli/src/commands/deploy.ts +++ b/packages/cli/src/commands/deploy.ts @@ -28,6 +28,9 @@ import { import { ConflictError, ValidationError } from '../rest/errors.js' import { stripUnsupportedDeployFields } from '../services/deploy-diff/legacy-payload.js' import { planChangeLines, reducePlanForAgent } from '../services/deploy-diff/plan-summary.js' +import { applyWriteBack, hasWritableChanges, planWriteBack } from '../services/write-back/plan.js' +import type { CommandAlternative } from '../helpers/command-preview.js' +import type { Project } from '../constructs/project.js' import { uploadSnapshots } from '../services/snapshot-service.js' import { BrowserCheckBundle } from '../constructs/browser-check-bundle.js' import { Runtime } from '../runtimes/index.js' @@ -58,6 +61,58 @@ function rejectsPreviewEraField (err: any): boolean { return message.includes('is not allowed') && PREVIEW_ERA_FIELDS.some(field => message.includes(field)) } +/** + * The further choice a terminal gets when the plan shows a resource edited + * outside the project and the code can take the edit: write the account's + * current values into the code and deploy nothing, so the user reviews the + * diff and deploys again rather than overwriting the edit. Only literal + * values of checks and groups can be written; everything else is listed + * with its reason. + */ +function writeBackAlternatives (diff: DiffEntry[], project: Project, command: Deploy): CommandAlternative[] { + if (!hasWritableChanges(diff, project)) { + return [] + } + return [{ + title: 'Update my code with the changes made in Checkly (deploys nothing)', + run: async () => { + const writeBack = await planWriteBack({ diff, project, cwd: process.cwd() }) + const oneLine = (text: string) => { + const first = text.split(/\r?\n/)[0] + return first.length === text.length ? first : `${first} …` + } + command.log() + if (writeBack.skipped.length > 0) { + command.log('Not updated (edit these by hand):') + for (const line of writeBack.skipped) { + command.log(` ${line}`) + } + } + if (writeBack.applied.length === 0) { + command.log('Nothing in the code could be updated automatically, so nothing was changed.') + command.log('Nothing was deployed.') + return + } + try { + await applyWriteBack(writeBack) + } catch (err: any) { + // The error names the files already rewritten, if any. + command.style.longError('Could not update your code.', err.message) + command.log('Nothing was deployed.') + command.exit(1) + } + // Reported once the files hold it, not as an intention. + command.log(`Updated ${writeBack.files.length === 1 ? '1 file' : `${writeBack.files.length} files`}:`) + for (const line of writeBack.applied) { + const note = line.replacesLocalEdit ? ' (replacing a local edit)' : '' + command.log(` ${line.file}: ${line.type} ${line.logicalId} ${line.property}: ` + + `${line.previous === undefined ? 'not set' : oneLine(line.previous)} -> ${oneLine(line.rendered)}${note}`) + } + command.log('Nothing was deployed. Review the changes, then run `checkly deploy` again.') + }, + }] +} + export default class Deploy extends AuthCommand { static coreCommand = true static hidden = false @@ -510,7 +565,14 @@ export default class Deploy extends AuthCommand { description: 'Deploy project to Checkly', changes: [...optionLines, ...planLines], ...plan !== undefined - ? { terminal: { plan: renderPlan, changes: optionLines }, question: 'Apply these changes?' } + ? { + terminal: { + plan: renderPlan, + changes: optionLines, + alternatives: writeBackAlternatives(plan.diff, project, this), + }, + question: 'Apply these changes?', + } : {}, // The token rides along in the echoed command, so the confirming run // deploys the plan that was shown here and refuses a different one. diff --git a/packages/cli/src/helpers/command-preview.ts b/packages/cli/src/helpers/command-preview.ts index c4fc9f90..6f4501ef 100644 --- a/packages/cli/src/helpers/command-preview.ts +++ b/packages/cli/src/helpers/command-preview.ts @@ -38,6 +38,18 @@ export type CommandPlanPreview = { export type CommandTerminalPreview = { plan: () => string changes: string[] + /** + * Things the user can choose to do instead of applying the command, offered + * as further choices of the prompt. Choosing one runs it and ends the + * command without applying anything. + */ + alternatives?: CommandAlternative[] +} + +export type CommandAlternative = { + /** The choice as the prompt lists it. */ + title: string + run: () => Promise } export type CommandPreview = { diff --git a/packages/cli/src/services/write-back/plan.ts b/packages/cli/src/services/write-back/plan.ts index 9664a51d..6cca4b2b 100644 --- a/packages/cli/src/services/write-back/plan.ts +++ b/packages/cli/src/services/write-back/plan.ts @@ -374,6 +374,26 @@ interface FileWork { edits: (LiteralEdit & { replacesLocalEdit: boolean, group?: string })[] } +/** + * Whether the plan holds at least one change the planner would try to + * write: a remote change on a construct of a known class, at a path the + * class's table covers. Cheap enough to decide whether to offer the + * write-back at all, without reading any file. + */ +export function hasWritableChanges (diff: readonly DiffEntry[], project: Project): boolean { + return diff.some(entry => { + if (entry.foldedInto !== undefined || entry.before === undefined) { + return false + } + const construct: Construct | undefined = project.data[entry.type as keyof ProjectData]?.[entry.logicalId] + const rules = construct === undefined ? undefined : RULES_BY_CLASS.get(construct.constructor as ConstructClass) + if (rules === undefined || construct?.checkFileAbsolutePath === undefined) { + return false + } + return candidates(new EntryContext(entry, []), rules).length > 0 + }) +} + export async function planWriteBack ({ diff, project, cwd }: WriteBackOptions): Promise { const skipped: string[] = [] const byFile = new Map() From 17c7a14936616a54e4583fc0e2012d551c4ce5c0 Mon Sep 17 00:00:00 2001 From: Simo Kinnunen Date: Tue, 22 Sep 2026 02:28:14 +0900 Subject: [PATCH 4/4] test(write-back): make the write tests hold on Windows [RED-985] The mode assertion compares against what the platform stored after chmod rather than a literal, since Windows keeps no group bits; the partial failure message is matched as text, since a Windows path's backslashes would be read as escapes by a RegExp. Co-Authored-By: Claude Fable 5.1 --- .../write-back/__tests__/plan.spec.ts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/services/write-back/__tests__/plan.spec.ts b/packages/cli/src/services/write-back/__tests__/plan.spec.ts index 7aeede50..81a05148 100644 --- a/packages/cli/src/services/write-back/__tests__/plan.spec.ts +++ b/packages/cli/src/services/write-back/__tests__/plan.spec.ts @@ -646,15 +646,18 @@ new ApiCheck('other', opts) describe('applyWriteBack', () => { it('keeps the file mode and writes through a symlink to its target', async () => { const real = path.join(dir, 'real.ts') - // Group-writable: a bit the usual umask would clear on a fresh file. + // Group-writable: a bit the usual umask would clear on a fresh file. What + // the platform actually stored is what must survive; on Windows, which + // keeps no group bits, this only checks the mode is unchanged. await fs.writeFile(real, 'old') await fs.chmod(real, 0o664) + const mode = (await fs.stat(real)).mode & 0o777 const link = path.join(dir, 'link.ts') await fs.symlink(real, link) await applyWriteBack({ files: [{ path: link, text: 'new', original: 'old' }], applied: [], skipped: [] }) expect(await fs.readFile(real, 'utf8')).toBe('new') expect((await fs.lstat(link)).isSymbolicLink()).toBe(true) - expect((await fs.stat(real)).mode & 0o777).toBe(0o664) + expect((await fs.stat(real)).mode & 0o777).toBe(mode) expect(await fs.readdir(dir)).toEqual(['link.ts', 'real.ts']) }) @@ -676,8 +679,16 @@ describe('applyWriteBack', () => { const good = path.join(dir, 'good.ts') await fs.writeFile(good, 'old', 'utf8') const bad = path.join(dir, 'missing', 'bad.ts') - await expect(applyWriteBack({ files: [{ path: good, text: 'new', original: 'old' }, { path: bad, text: 'x', original: '' }], applied: [], skipped: [] })) - .rejects.toThrow(new RegExp(`Could not write ${bad}: .*Already updated: ${good}\\.`)) + // Paths are matched as text: a Windows path has backslashes a RegExp would read as escapes. + const run = applyWriteBack({ + files: [{ path: good, text: 'new', original: 'old' }, { path: bad, text: 'x', original: '' }], + applied: [], + skipped: [], + }) + await expect(run).rejects.toThrow('Could not write ') + const failure = await run.catch((err: Error) => err.message) + expect(failure).toContain(`Could not write ${bad}: `) + expect(failure).toContain(`Already updated: ${good}.`) expect(await fs.readFile(good, 'utf8')).toBe('new') expect(await fs.readdir(dir)).toEqual(['good.ts']) })