From df6589b881fb6a3966c665a1a647ba430032b0ce Mon Sep 17 00:00:00 2001 From: Spiros Martzoukos Date: Tue, 15 Sep 2026 13:13:53 +0300 Subject: [PATCH 1/2] feat(cli): report each construct's source file on deploy `checkly deploy` now sends an optional `sourceFile` on every resource envelope: the path of the file that declares the construct, relative to the git repository root with posix separators. Checkly persists it so it can open pull requests against the right file without scanning the repo. - Constructs declared in `checkly.config.ts` report the config file's path. - Outside a git repository, or for files outside it, the field is omitted. - The repo root is resolved via a new `getGitRepoRoot()` and deliberately kept out of `GitInformation`, which is sent to the API verbatim. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WwZUrSrvYbmAseT1wCvk8e --- .../__tests__/confirm-flow-deploy.spec.ts | 1 + .../deploy-config-diagnostics.spec.ts | 1 + .../__tests__/deploy-source-file.spec.ts | 165 ++++++++++++++++++ packages/cli/src/commands/deploy.ts | 5 +- .../__tests__/project-bundle.spec.ts | 113 ++++++++++++ packages/cli/src/constructs/project-bundle.ts | 90 +++++++--- packages/cli/src/rest/projects.ts | 6 + .../__tests__/checkly-config-loader.spec.ts | 40 ++++- .../cli/src/services/__tests__/util.spec.ts | 15 ++ .../cli/src/services/checkly-config-loader.ts | 11 +- packages/cli/src/services/util.ts | 11 ++ 11 files changed, 432 insertions(+), 26 deletions(-) create mode 100644 packages/cli/src/commands/__tests__/deploy-source-file.spec.ts create mode 100644 packages/cli/src/constructs/__tests__/project-bundle.spec.ts 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 9bc4179c2..a7c98b618 100644 --- a/packages/cli/src/commands/__tests__/confirm-flow-deploy.spec.ts +++ b/packages/cli/src/commands/__tests__/confirm-flow-deploy.spec.ts @@ -37,6 +37,7 @@ vi.mock('../../services/util', () => ({ configFilenames: ['checkly.config.ts'], }), getGitInformation: vi.fn(), + getGitRepoRoot: vi.fn(), })) vi.mock('prompts', () => ({ diff --git a/packages/cli/src/commands/__tests__/deploy-config-diagnostics.spec.ts b/packages/cli/src/commands/__tests__/deploy-config-diagnostics.spec.ts index cc3063065..5cb36b127 100644 --- a/packages/cli/src/commands/__tests__/deploy-config-diagnostics.spec.ts +++ b/packages/cli/src/commands/__tests__/deploy-config-diagnostics.spec.ts @@ -23,6 +23,7 @@ vi.mock('../../services/util', () => ({ configFilenames: ['checkly.config.ts'], }), getGitInformation: vi.fn(), + getGitRepoRoot: vi.fn(), })) import { loadChecklyConfig } from '../../services/checkly-config-loader.js' diff --git a/packages/cli/src/commands/__tests__/deploy-source-file.spec.ts b/packages/cli/src/commands/__tests__/deploy-source-file.spec.ts new file mode 100644 index 000000000..6d27b44dd --- /dev/null +++ b/packages/cli/src/commands/__tests__/deploy-source-file.spec.ts @@ -0,0 +1,165 @@ +import path from 'node:path' + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('../../helpers/cli-mode', () => ({ + detectCliMode: vi.fn(() => 'agent'), +})) + +vi.mock('../../rest/api', () => ({ + runtimes: { getAll: vi.fn().mockResolvedValue([]) }, + projects: { deploy: vi.fn().mockResolvedValue({ data: { diff: [] } }) }, + validateAuthentication: vi.fn().mockResolvedValue({ name: 'Test Account' }), +})) + +vi.mock('../../services/checkly-config-loader', async () => { + const { Diagnostics } = await import('../../constructs/diagnostics.js') + return { + loadChecklyConfig: vi.fn().mockResolvedValue({ + config: { + logicalId: 'my-project', + projectName: 'My Project', + checks: {}, + }, + constructs: [], + diagnostics: new Diagnostics(), + }), + resolveDependencyCacheVersion: vi.fn(), + } +}) + +vi.mock('../../services/project-parser', () => ({ + parseProject: vi.fn(), +})) + +vi.mock('../../services/util', async importOriginal => ({ + ...await importOriginal(), + splitConfigFilePath: vi.fn().mockReturnValue({ + configDirectory: '.', + configFilenames: ['checkly.config.ts'], + }), + getGitInformation: vi.fn(), + getGitRepoRoot: vi.fn(), +})) + +vi.mock('../../services/check-parser/bundler', () => ({ + Bundler: { + createForWorkspace: vi.fn().mockResolvedValue({ + isEmpty: true, + updateMarker: vi.fn(), + finalize: vi.fn().mockResolvedValue({ archiveFile: 'bundle.tgz', store: vi.fn() }), + }), + }, +})) + +import * as api from '../../rest/api.js' +import { parseProject } from '../../services/project-parser.js' +import { getGitInformation, getGitRepoRoot } from '../../services/util.js' +import { Ok } from '../../services/check-parser/package-files/result.js' +import { EmailAlertChannel } from '../../constructs/email-alert-channel.js' +import { Project } from '../../constructs/project.js' +import { Session } from '../../constructs/session.js' +import { AuthCommand } from '../authCommand.js' +import Deploy from '../deploy.js' + +const repoRoot = path.resolve('/home/user/repo') + +function createCommandContext () { + return { + parse: vi.fn().mockResolvedValue({ + flags: { + 'force': true, + 'preview': true, + 'output': false, + 'verbose': false, + 'config': undefined, + 'schedule-on-deploy': true, + 'verify-runtime-dependencies': true, + 'debug-bundle': false, + 'debug-bundle-output-file': './debug-bundle.json', + }, + metadata: { flags: {} }, + }), + log: vi.fn(), + exit: vi.fn((code: number) => { + throw new Error(`EXIT_${code}`) + }), + style: { + outputFormat: undefined, + diagnostics: vi.fn(), + actionStart: vi.fn(), + actionSuccess: vi.fn(), + actionFailure: vi.fn(), + actionStatus: vi.fn(), + longError: vi.fn(), + longWarning: vi.fn(), + longInfo: vi.fn(), + shortError: vi.fn(), + }, + validateProject: (AuthCommand.prototype as any).validateProject, + formatPreview: (Deploy.prototype as any).formatPreview, + constructor: Deploy, + account: { name: 'Test Account', runtimeId: 'runtime-default' }, + } +} + +function declareProject () { + Session.reset() + Session.workspace = Ok({} as any) + const project = new Project('my-project', { name: 'My Project' }) + Session.project = project + + Session.checkFileAbsolutePath = path.join(repoRoot, 'src', 'alerts.ts') + new EmailAlertChannel('in-repo', { address: 'alerts@example.com' }) + Session.checkFileAbsolutePath = path.resolve('/home/user/elsewhere/alerts.ts') + new EmailAlertChannel('outside-repo', { address: 'alerts@example.com' }) + Session.checkFileAbsolutePath = undefined + + vi.mocked(parseProject).mockResolvedValue(project) + return project +} + +function deployedResources () { + expect(api.projects.deploy).toHaveBeenCalledOnce() + const [payload] = vi.mocked(api.projects.deploy).mock.calls[0] + return payload.resources +} + +describe('deploy source files', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(getGitInformation).mockReturnValue(null) + declareProject() + }) + + afterEach(() => { + Session.reset() + }) + + it('sends each resource\'s source file relative to the git repository root', async () => { + vi.mocked(getGitRepoRoot).mockReturnValue(repoRoot) + + await Deploy.prototype.run.call(createCommandContext() as any) + + const resources = deployedResources() + expect(resources).toEqual(expect.arrayContaining([ + expect.objectContaining({ logicalId: 'in-repo', type: 'alert-channel', sourceFile: 'src/alerts.ts' }), + ])) + const outside = resources.find(resource => resource.logicalId === 'outside-repo') + expect(outside).toBeDefined() + expect(outside).not.toHaveProperty('sourceFile') + expect(outside!.payload).not.toHaveProperty('sourceFile') + }) + + it('omits source files outside a git repository', async () => { + vi.mocked(getGitRepoRoot).mockReturnValue(undefined) + + await Deploy.prototype.run.call(createCommandContext() as any) + + const resources = deployedResources() + expect(resources).toHaveLength(2) + for (const resource of resources) { + expect(resource).not.toHaveProperty('sourceFile') + } + }) +}) diff --git a/packages/cli/src/commands/deploy.ts b/packages/cli/src/commands/deploy.ts index c7cd45956..65a723e3c 100644 --- a/packages/cli/src/commands/deploy.ts +++ b/packages/cli/src/commands/deploy.ts @@ -13,7 +13,7 @@ import { StatusPageV3Component, StatusPageV3AutomationRule, } from '../constructs/index.js' import chalk from 'chalk' -import { splitConfigFilePath, getGitInformation } from '../services/util.js' +import { splitConfigFilePath, getGitInformation, getGitRepoRoot } from '../services/util.js' import commonMessages from '../messages/common-messages.js' import { forceFlag } from '../helpers/flags.js' import { ProjectDeployResponse, ProjectDeployCancelledError } from '../rest/projects.js' @@ -186,6 +186,7 @@ export default class Deploy extends AuthCommand { playwrightChecks: checklyConfig.checks?.playwrightChecks, }) const repoInfo = getGitInformation(project.repoUrl) + const repoRoot = getGitRepoRoot() this.style.actionSuccess() @@ -254,7 +255,7 @@ export default class Deploy extends AuthCommand { } } - const projectPayload = projectBundle.synthesize() + const projectPayload = projectBundle.synthesize({ repoRoot }) if (!projectPayload.resources.length) { if (preview) { this.log('\nNo checks were detected. More information on how to set up a Checkly CLI project is available at https://checklyhq.com/docs/cli/.\n') diff --git a/packages/cli/src/constructs/__tests__/project-bundle.spec.ts b/packages/cli/src/constructs/__tests__/project-bundle.spec.ts new file mode 100644 index 000000000..23ee8cc25 --- /dev/null +++ b/packages/cli/src/constructs/__tests__/project-bundle.spec.ts @@ -0,0 +1,113 @@ +import path from 'node:path' + +import { describe, it, expect, beforeEach } from 'vitest' + +import { EmailAlertChannel } from '../email-alert-channel.js' +import { Project } from '../project.js' +import { resolveSourceFile } from '../project-bundle.js' +import { Session } from '../session.js' +import { Bundler } from '../../services/check-parser/bundler.js' + +const repoRoot = path.resolve('/home/user/repo') + +async function bundleProject () { + const bundler = await Bundler.create({ cacheHash: 'foo' }) + return Session.project!.bundle(bundler) +} + +describe('resolveSourceFile()', () => { + it('returns the file relative to the repository root', () => { + expect(resolveSourceFile(repoRoot, path.join(repoRoot, 'src', '__checks__', 'api.check.ts'))) + .toBe('src/__checks__/api.check.ts') + }) + + it('is undefined without a repository root', () => { + expect(resolveSourceFile(undefined, path.join(repoRoot, 'api.check.ts'))).toBeUndefined() + }) + + it('is undefined without a declaring file', () => { + expect(resolveSourceFile(repoRoot, undefined)).toBeUndefined() + }) + + it('is undefined for a file outside the repository', () => { + expect(resolveSourceFile(repoRoot, path.resolve('/home/user/elsewhere/api.check.ts'))).toBeUndefined() + expect(resolveSourceFile(repoRoot, repoRoot)).toBeUndefined() + }) + + it('uses posix separators for Windows paths', () => { + expect(resolveSourceFile('C:\\repo', 'C:\\repo\\src\\api.check.ts', path.win32)) + .toBe('src/api.check.ts') + }) + + it('is undefined for a file on another Windows drive', () => { + expect(resolveSourceFile('C:\\repo', 'D:\\other\\api.check.ts', path.win32)).toBeUndefined() + }) +}) + +describe('ProjectBundle.synthesize()', () => { + beforeEach(() => { + Session.reset() + Session.project = new Project('project-id', { + name: 'Test Project', + repoUrl: 'https://github.com/checkly/checkly-cli', + }) + }) + + function declareAlertChannel (logicalId: string, checkFileAbsolutePath: string | undefined) { + Session.checkFileAbsolutePath = checkFileAbsolutePath + try { + return new EmailAlertChannel(logicalId, { address: 'alerts@example.com' }) + } finally { + Session.checkFileAbsolutePath = undefined + } + } + + it('reports each resource\'s source file relative to the repository root', async () => { + declareAlertChannel('email', path.join(repoRoot, 'src', 'alerts', 'email.ts')) + + const { resources } = (await bundleProject()).synthesize({ repoRoot }) + + expect(resources).toEqual([ + expect.objectContaining({ logicalId: 'email', sourceFile: 'src/alerts/email.ts' }), + ]) + }) + + it('reports the config file for constructs declared in checkly.config.ts', async () => { + declareAlertChannel('email', path.join(repoRoot, 'checkly.config.ts')) + + const { resources } = (await bundleProject()).synthesize({ repoRoot }) + + expect(resources).toEqual([ + expect.objectContaining({ logicalId: 'email', sourceFile: 'checkly.config.ts' }), + ]) + }) + + it('omits sourceFile without a repository root', async () => { + declareAlertChannel('email', path.join(repoRoot, 'src', 'email.ts')) + + const { resources } = (await bundleProject()).synthesize() + + expect(resources).toHaveLength(1) + expect(resources[0]).not.toHaveProperty('sourceFile') + }) + + it('omits sourceFile for constructs declared outside the repository', async () => { + declareAlertChannel('outside', path.resolve('/home/user/elsewhere/email.ts')) + declareAlertChannel('unknown', undefined) + + const { resources } = (await bundleProject()).synthesize({ repoRoot }) + + expect(resources).toHaveLength(2) + for (const resource of resources) { + expect(resource).not.toHaveProperty('sourceFile') + } + }) + + it('keeps sourceFile on the envelope rather than in the payload', async () => { + declareAlertChannel('email', path.join(repoRoot, 'src', 'email.ts')) + + const { resources } = (await bundleProject()).synthesize({ repoRoot }) + + expect(resources[0].payload).not.toHaveProperty('sourceFile') + }) +}) diff --git a/packages/cli/src/constructs/project-bundle.ts b/packages/cli/src/constructs/project-bundle.ts index 228ff7603..dc6a8c1df 100644 --- a/packages/cli/src/constructs/project-bundle.ts +++ b/packages/cli/src/constructs/project-bundle.ts @@ -1,5 +1,7 @@ +import * as path from 'node:path' import { Bundle, Construct } from './construct.js' import { Project, Resources } from './project.js' +import { pathToPosix } from '../services/util.js' export type ResourceDataBundle = { construct: T @@ -10,6 +12,41 @@ export type ProjectDataBundle = { [x in keyof Resources]: Record> } +export interface ProjectSynthesizeOptions { + /** + * Absolute path of the git repository root. When set, every resource + * reports the file that declares it as a `sourceFile` relative to this + * root, so Checkly can open pull requests against the right file. + */ + repoRoot?: string +} + +/** + * The construct's declaring file relative to the repository root, with posix + * separators; `undefined` when either path is unknown or the file lives + * outside the repository. + * + * @param platformPath this is for testing purposes only so we can exercise + * Windows path handling on Linux / Darwin + */ +export function resolveSourceFile ( + repoRoot: string | undefined, + checkFileAbsolutePath: string | undefined, + platformPath: path.PlatformPath = path, +): string | undefined { + if (!repoRoot || !checkFileAbsolutePath) { + return undefined + } + const relativePath = platformPath.relative(repoRoot, checkFileAbsolutePath) + // An empty result means the file is the root itself; a leading `..` or an + // absolute result (a different Windows drive) means it lives outside the + // repository. Neither can be opened as a file in the repository. + if (!relativePath || relativePath.startsWith('..') || platformPath.isAbsolute(relativePath)) { + return undefined + } + return pathToPosix(relativePath, platformPath.sep) +} + export class ProjectBundle implements Bundle { project: Project data: ProjectDataBundle @@ -19,40 +56,49 @@ export class ProjectBundle implements Bundle { this.data = data } - private synthesizeRecord (record: Record>) { + private synthesizeRecord ( + record: Record>, + { repoRoot }: ProjectSynthesizeOptions, + ) { return Object.entries(record) - .map(([key, { construct, bundle }]) => ({ - logicalId: key, - type: construct.type, - physicalId: construct.physicalId, - member: construct.member, - payload: bundle.synthesize(), - })) + .map(([key, { construct, bundle }]) => { + const sourceFile = resolveSourceFile(repoRoot, construct.checkFileAbsolutePath) + return { + logicalId: key, + type: construct.type, + physicalId: construct.physicalId, + member: construct.member, + payload: bundle.synthesize(), + // Only present when known, so older backends see an unchanged + // envelope. + ...(sourceFile !== undefined ? { sourceFile } : {}), + } + }) } - synthesize () { + synthesize (options: ProjectSynthesizeOptions = {}) { return { ...this.project.synthesize(), resources: [ // The order in which resources are defined here is important. If // resource A may include references to resource B, it should occur // later than resource B. - ...this.synthesizeRecord(this.data['status-page-service']), - ...this.synthesizeRecord(this.data['status-page']), + ...this.synthesizeRecord(this.data['status-page-service'], options), + ...this.synthesizeRecord(this.data['status-page'], options), // v3: components reference their page (and parent group), rules // reference the page and components. Declaration order keeps parents // before children within components. - ...this.synthesizeRecord(this.data['status-page-component']), - ...this.synthesizeRecord(this.data['status-page-automation-rule']), - ...this.synthesizeRecord(this.data['check-group']), - ...this.synthesizeRecord(this.data.check), - ...this.synthesizeRecord(this.data['alert-channel']), - ...this.synthesizeRecord(this.data['alert-channel-subscription']), - ...this.synthesizeRecord(this.data['maintenance-window']), - ...this.synthesizeRecord(this.data['private-location']), - ...this.synthesizeRecord(this.data['private-location-check-assignment']), - ...this.synthesizeRecord(this.data['private-location-group-assignment']), - ...this.synthesizeRecord(this.data.dashboard), + ...this.synthesizeRecord(this.data['status-page-component'], options), + ...this.synthesizeRecord(this.data['status-page-automation-rule'], options), + ...this.synthesizeRecord(this.data['check-group'], options), + ...this.synthesizeRecord(this.data.check, options), + ...this.synthesizeRecord(this.data['alert-channel'], options), + ...this.synthesizeRecord(this.data['alert-channel-subscription'], options), + ...this.synthesizeRecord(this.data['maintenance-window'], options), + ...this.synthesizeRecord(this.data['private-location'], options), + ...this.synthesizeRecord(this.data['private-location-check-assignment'], options), + ...this.synthesizeRecord(this.data['private-location-group-assignment'], options), + ...this.synthesizeRecord(this.data.dashboard, options), ], } } diff --git a/packages/cli/src/rest/projects.ts b/packages/cli/src/rest/projects.ts index abb7b1cb6..873554ea5 100644 --- a/packages/cli/src/rest/projects.ts +++ b/packages/cli/src/rest/projects.ts @@ -26,6 +26,12 @@ export interface ResourceSync { type: string member: boolean payload: any + /** + * The file that declares the construct, relative to the git repository + * root with posix separators. Absent outside a git repository or when the + * file lives outside the repository. + */ + sourceFile?: string } export interface AlertChannelFriendResource { diff --git a/packages/cli/src/services/__tests__/checkly-config-loader.spec.ts b/packages/cli/src/services/__tests__/checkly-config-loader.spec.ts index baa7295b5..1e56beba3 100644 --- a/packages/cli/src/services/__tests__/checkly-config-loader.spec.ts +++ b/packages/cli/src/services/__tests__/checkly-config-loader.spec.ts @@ -1,10 +1,11 @@ import path from 'node:path' -import { describe, it, expect } from 'vitest' +import { describe, it, expect, vi, afterEach } from 'vitest' import { loadChecklyConfig, defaultFilenames, resolveDependencyCacheVersion } from '../checkly-config-loader.js' import { InvalidConfigError } from '../config-diagnostics.js' import { splitConfigFilePath } from '../util.js' +import { Session } from '../../constructs/session.js' const configDir = path.join(__dirname, 'fixtures', 'configs') @@ -453,6 +454,43 @@ describe('loadChecklyConfig()', () => { ['runner-registries-literal-token.js'], )).rejects.toThrow(/must be exactly one environment variable reference in \$\{VAR\} syntax/) }) + describe('construct source file attribution', () => { + afterEach(() => { + vi.restoreAllMocks() + }) + it('loads the config file with its absolute path as the current check file', async () => { + const filename = 'good-config.ts' + let checkFileAbsolutePath: string | undefined + let checkFilePath: string | undefined + const loadFile = vi.spyOn(Session, 'loadFile').mockImplementation(() => { + checkFileAbsolutePath = Session.checkFileAbsolutePath + checkFilePath = Session.checkFilePath + return Promise.resolve({ logicalId: 'test', projectName: 'Test' }) + }) + + await loadChecklyConfig(configDir, [filename]) + + expect(loadFile).toHaveBeenCalledOnce() + expect(checkFileAbsolutePath).toBe(path.join(configDir, filename)) + expect(path.isAbsolute(checkFileAbsolutePath!)).toBe(true) + // Session.checkFilePath drives `checkly test --files` filtering and is + // reserved for check files. + expect(checkFilePath).toBeUndefined() + }) + it('clears the current check file after loading the config, even on failure', async () => { + const filename = 'good-config.ts' + let checkFileAbsolutePath: string | undefined + vi.spyOn(Session, 'loadFile').mockImplementation(() => { + checkFileAbsolutePath = Session.checkFileAbsolutePath + return Promise.reject(new Error('boom')) + }) + + await expect(loadChecklyConfig(configDir, [filename])).rejects.toThrow('boom') + + expect(checkFileAbsolutePath).toBe(path.join(configDir, filename)) + expect(Session.checkFileAbsolutePath).toBeUndefined() + }) + }) it('config from absolute path', async () => { const filename = 'good-config.ts' const configFile = `./fixtures/configs/${filename}` diff --git a/packages/cli/src/services/__tests__/util.spec.ts b/packages/cli/src/services/__tests__/util.spec.ts index 954b3dfd8..116406073 100644 --- a/packages/cli/src/services/__tests__/util.spec.ts +++ b/packages/cli/src/services/__tests__/util.spec.ts @@ -3,6 +3,7 @@ import { afterEach, beforeEach, describe, it, expect } from 'vitest' import { getGitInformation, + getGitRepoRoot, pathToPosix, isFileSync, } from '../util.js' @@ -71,6 +72,20 @@ describe('util', () => { }) }) + describe('getGitRepoRoot()', () => { + it('returns the absolute root of the enclosing git repository', () => { + const root = getGitRepoRoot() + expect(root).toBeDefined() + expect(path.isAbsolute(root!)).toBe(true) + expect(isFileSync(path.join(root!, '.git'))).toBe(true) + }) + + it('is not part of the git information sent to the API', () => { + process.env.CHECKLY_REPO_SHA = 'abc123' + expect(getGitInformation()).not.toHaveProperty('repoRoot') + }) + }) + describe('getGitInformation()', () => { it('should not include GitHub metadata by default', () => { process.env.CHECKLY_REPO_SHA = 'abc123' diff --git a/packages/cli/src/services/checkly-config-loader.ts b/packages/cli/src/services/checkly-config-loader.ts index ce67d7568..ddd06e6fd 100644 --- a/packages/cli/src/services/checkly-config-loader.ts +++ b/packages/cli/src/services/checkly-config-loader.ts @@ -492,7 +492,16 @@ export async function loadChecklyConfig ( } catch { continue } - config = await Session.loadFile(filePath) + // Constructs declared in the config file capture the file they come + // from, like constructs in check files do, so deploys can report it. + // Only the absolute path is set: Session.checkFilePath drives + // `checkly test --files` filtering and must stay unset here. + Session.checkFileAbsolutePath = path.resolve(filePath) + try { + config = await Session.loadFile(filePath) + } finally { + Session.checkFileAbsolutePath = undefined + } configFileName = path.relative(process.cwd(), filePath) break } diff --git a/packages/cli/src/services/util.ts b/packages/cli/src/services/util.ts index 524530d13..e64b9098b 100644 --- a/packages/cli/src/services/util.ts +++ b/packages/cli/src/services/util.ts @@ -174,6 +174,17 @@ export function getGitInformation (repoUrl?: string): GitInformation | null { return gitInformation } +/** + * The absolute path of the git repository root the CLI runs in, or + * `undefined` outside a repository. Kept separate from GitInformation, which + * is sent to the API as `repoInfo` verbatim and must not carry local + * filesystem paths. + */ +export function getGitRepoRoot (): string | undefined { + const { root } = gitRepoInfo() + return root || undefined +} + export function getCiInformation (): CiInformation { return { environment: process.env.CHECKLY_TEST_ENVIRONMENT ?? null, From 0f2897bc4ad0fb568b84805f9d67794e4fb93872 Mon Sep 17 00:00:00 2001 From: Spiros Martzoukos Date: Tue, 15 Sep 2026 15:00:13 +0300 Subject: [PATCH 2/2] fix(cli): resolve the repo root from the worktree and harden sourceFile - getGitRepoRoot walks up to the nearest .git entry instead of using git-repo-info's root, which reports the main checkout inside a linked worktree and would attribute files to the wrong tree. - resolveSourceFile only rejects a real `..` segment and honours the injected platform separator end to end. - loadChecklyConfig restores the previously active check file rather than clearing it; a test pins that config-declared constructs record the config file and resolve relative paths against it. - sourceFile moves to DeployResourceSync so the shared ResourceSync used by the import plan response does not advertise it. - util.spec asserts the exact key set of repoInfo. Co-Authored-By: Claude Opus 5 --- .../__tests__/project-bundle.spec.ts | 5 +++ packages/cli/src/constructs/project-bundle.ts | 12 ++--- packages/cli/src/rest/projects.ts | 13 +++++- .../__tests__/checkly-config-loader.spec.ts | 36 +++++++++++++++ .../cli/src/services/__tests__/util.spec.ts | 45 ++++++++++++++++++- .../cli/src/services/checkly-config-loader.ts | 8 +++- packages/cli/src/services/util.ts | 21 +++++++-- 7 files changed, 126 insertions(+), 14 deletions(-) diff --git a/packages/cli/src/constructs/__tests__/project-bundle.spec.ts b/packages/cli/src/constructs/__tests__/project-bundle.spec.ts index 23ee8cc25..f69a3799b 100644 --- a/packages/cli/src/constructs/__tests__/project-bundle.spec.ts +++ b/packages/cli/src/constructs/__tests__/project-bundle.spec.ts @@ -34,6 +34,11 @@ describe('resolveSourceFile()', () => { expect(resolveSourceFile(repoRoot, repoRoot)).toBeUndefined() }) + it('keeps a repository entry whose name merely starts with two dots', () => { + expect(resolveSourceFile(repoRoot, path.join(repoRoot, '..dotted', 'api.check.ts'))) + .toBe('..dotted/api.check.ts') + }) + it('uses posix separators for Windows paths', () => { expect(resolveSourceFile('C:\\repo', 'C:\\repo\\src\\api.check.ts', path.win32)) .toBe('src/api.check.ts') diff --git a/packages/cli/src/constructs/project-bundle.ts b/packages/cli/src/constructs/project-bundle.ts index dc6a8c1df..5ea354127 100644 --- a/packages/cli/src/constructs/project-bundle.ts +++ b/packages/cli/src/constructs/project-bundle.ts @@ -1,7 +1,6 @@ import * as path from 'node:path' import { Bundle, Construct } from './construct.js' import { Project, Resources } from './project.js' -import { pathToPosix } from '../services/util.js' export type ResourceDataBundle = { construct: T @@ -38,13 +37,14 @@ export function resolveSourceFile ( return undefined } const relativePath = platformPath.relative(repoRoot, checkFileAbsolutePath) - // An empty result means the file is the root itself; a leading `..` or an - // absolute result (a different Windows drive) means it lives outside the - // repository. Neither can be opened as a file in the repository. - if (!relativePath || relativePath.startsWith('..') || platformPath.isAbsolute(relativePath)) { + // An empty result means the file is the root itself; a leading `..` + // segment or an absolute result (a different Windows drive) means it lives + // outside the repository. Neither can be opened as a file in the repository. + const escapesRoot = relativePath === '..' || relativePath.startsWith(`..${platformPath.sep}`) + if (!relativePath || escapesRoot || platformPath.isAbsolute(relativePath)) { return undefined } - return pathToPosix(relativePath, platformPath.sep) + return relativePath.split(platformPath.sep).join(path.posix.sep) } export class ProjectBundle implements Bundle { diff --git a/packages/cli/src/rest/projects.ts b/packages/cli/src/rest/projects.ts index 873554ea5..9665dc04d 100644 --- a/packages/cli/src/rest/projects.ts +++ b/packages/cli/src/rest/projects.ts @@ -26,10 +26,19 @@ export interface ResourceSync { type: string member: boolean payload: any +} + +/** + * A resource as sent by `checkly deploy`. The import plan API returns plain + * ResourceSync entries and never carries `sourceFile`. + */ +export interface DeployResourceSync extends ResourceSync { /** * The file that declares the construct, relative to the git repository * root with posix separators. Absent outside a git repository or when the - * file lives outside the repository. + * file lives outside the repository. A hint: constructs instantiated in a + * module imported by a check file or checkly.config.ts report the + * importing file, so the backend should verify before editing. */ sourceFile?: string } @@ -87,7 +96,7 @@ export interface AuxiliaryResourceSync { export interface ProjectSync { project: Project sharedFiles?: SharedFile[] - resources: Array + resources: Array repoInfo: GitInformation | null } diff --git a/packages/cli/src/services/__tests__/checkly-config-loader.spec.ts b/packages/cli/src/services/__tests__/checkly-config-loader.spec.ts index 1e56beba3..dbc411165 100644 --- a/packages/cli/src/services/__tests__/checkly-config-loader.spec.ts +++ b/packages/cli/src/services/__tests__/checkly-config-loader.spec.ts @@ -6,6 +6,7 @@ import { loadChecklyConfig, defaultFilenames, resolveDependencyCacheVersion } fr import { InvalidConfigError } from '../config-diagnostics.js' import { splitConfigFilePath } from '../util.js' import { Session } from '../../constructs/session.js' +import { CheckGroupV1 } from '../../constructs/check-group-v1.js' const configDir = path.join(__dirname, 'fixtures', 'configs') @@ -490,6 +491,41 @@ describe('loadChecklyConfig()', () => { expect(checkFileAbsolutePath).toBe(path.join(configDir, filename)) expect(Session.checkFileAbsolutePath).toBeUndefined() }) + it('restores a previously active check file instead of clearing it', async () => { + const filename = 'good-config.ts' + const previous = path.join(configDir, 'some.check.ts') + Session.checkFileAbsolutePath = previous + vi.spyOn(Session, 'loadFile').mockResolvedValue({ logicalId: 'test', projectName: 'Test' }) + try { + await loadChecklyConfig(configDir, [filename]) + expect(Session.checkFileAbsolutePath).toBe(previous) + } finally { + Session.checkFileAbsolutePath = undefined + } + }) + it('resolves relative paths on config-declared constructs against the config file', async () => { + // Before the loader set the check file, a CheckGroup with a testMatch + // in checkly.config.ts crashed on `path.dirname(undefined)`. It now + // globs relative to the config directory and records the config file + // as its declaring file. + const filename = 'good-config.ts' + const configPath = path.join(configDir, filename) + vi.spyOn(Session, 'loadFile').mockImplementation(() => { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const group = new CheckGroupV1('config-group', { + name: 'Config group', + locations: ['us-east-1'], + browserChecks: { testMatch: '__no_such_dir__/*.spec.ts' }, + }) + return Promise.resolve({ logicalId: 'test', projectName: 'Test' }) + }) + + const { constructs } = await loadChecklyConfig(configDir, [filename]) + + expect(constructs).toHaveLength(1) + expect(constructs[0]).toBeInstanceOf(CheckGroupV1) + expect(constructs[0].checkFileAbsolutePath).toBe(configPath) + }) }) it('config from absolute path', async () => { const filename = 'good-config.ts' diff --git a/packages/cli/src/services/__tests__/util.spec.ts b/packages/cli/src/services/__tests__/util.spec.ts index 116406073..dceae72bc 100644 --- a/packages/cli/src/services/__tests__/util.spec.ts +++ b/packages/cli/src/services/__tests__/util.spec.ts @@ -1,3 +1,5 @@ +import fs from 'node:fs' +import os from 'node:os' import path from 'node:path' import { afterEach, beforeEach, describe, it, expect } from 'vitest' @@ -73,6 +75,16 @@ describe('util', () => { }) describe('getGitRepoRoot()', () => { + let tmpDir: string + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'checkly-git-root-')) + }) + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }) + }) + it('returns the absolute root of the enclosing git repository', () => { const root = getGitRepoRoot() expect(root).toBeDefined() @@ -80,9 +92,40 @@ describe('util', () => { expect(isFileSync(path.join(root!, '.git'))).toBe(true) }) + it('walks up from a nested directory to the `.git` directory', () => { + fs.mkdirSync(path.join(tmpDir, '.git')) + const nested = path.join(tmpDir, 'src', '__checks__') + fs.mkdirSync(nested, { recursive: true }) + expect(getGitRepoRoot(nested)).toBe(tmpDir) + }) + + it('treats a linked worktree as its own root, not the main checkout', () => { + // Layout git produces for `git worktree add .claude/worktrees/feat`: + // the worktree holds a `.git` *file* pointing at a gitdir inside the + // main checkout's `.git`, which in turn carries a `commondir`. + const mainGitDir = path.join(tmpDir, '.git') + const worktreeGitDir = path.join(mainGitDir, 'worktrees', 'feat') + fs.mkdirSync(worktreeGitDir, { recursive: true }) + fs.writeFileSync(path.join(worktreeGitDir, 'commondir'), '../..\n') + const worktree = path.join(tmpDir, '.claude', 'worktrees', 'feat') + const checksDir = path.join(worktree, 'src') + fs.mkdirSync(checksDir, { recursive: true }) + fs.writeFileSync(path.join(worktree, '.git'), `gitdir: ${worktreeGitDir}\n`) + + expect(getGitRepoRoot(checksDir)).toBe(worktree) + }) + + it('returns undefined outside a git repository', () => { + expect(getGitRepoRoot(tmpDir)).toBeUndefined() + }) + it('is not part of the git information sent to the API', () => { process.env.CHECKLY_REPO_SHA = 'abc123' - expect(getGitInformation()).not.toHaveProperty('repoRoot') + // Exact key set: any new field on repoInfo must be a deliberate choice, + // since the object goes to the API verbatim. + expect(Object.keys(getGitInformation()!).sort()).toEqual([ + 'branchName', 'commitId', 'commitMessage', 'commitOwner', 'repoUrl', + ]) }) }) diff --git a/packages/cli/src/services/checkly-config-loader.ts b/packages/cli/src/services/checkly-config-loader.ts index ddd06e6fd..f7f3fbe63 100644 --- a/packages/cli/src/services/checkly-config-loader.ts +++ b/packages/cli/src/services/checkly-config-loader.ts @@ -493,14 +493,18 @@ export async function loadChecklyConfig ( continue } // Constructs declared in the config file capture the file they come - // from, like constructs in check files do, so deploys can report it. + // from, like constructs in check files do. Deploys report it as the + // construct's sourceFile, and relative paths on config-declared + // constructs (a check group's testMatch, a setup script entrypoint) + // resolve against the config file's directory instead of failing. // Only the absolute path is set: Session.checkFilePath drives // `checkly test --files` filtering and must stay unset here. + const previousCheckFileAbsolutePath = Session.checkFileAbsolutePath Session.checkFileAbsolutePath = path.resolve(filePath) try { config = await Session.loadFile(filePath) } finally { - Session.checkFileAbsolutePath = undefined + Session.checkFileAbsolutePath = previousCheckFileAbsolutePath } configFileName = path.relative(process.cwd(), filePath) break diff --git a/packages/cli/src/services/util.ts b/packages/cli/src/services/util.ts index e64b9098b..98147ab79 100644 --- a/packages/cli/src/services/util.ts +++ b/packages/cli/src/services/util.ts @@ -179,10 +179,25 @@ export function getGitInformation (repoUrl?: string): GitInformation | null { * `undefined` outside a repository. Kept separate from GitInformation, which * is sent to the API as `repoInfo` verbatim and must not carry local * filesystem paths. + * + * Walks up from `startDir` to the nearest `.git` entry rather than using + * `git-repo-info`'s `root`: in a linked worktree that library reports the + * main checkout, and files would then be attributed relative to the wrong + * tree. A `.git` *file* (worktree or submodule) counts as a root just like + * a `.git` directory does. */ -export function getGitRepoRoot (): string | undefined { - const { root } = gitRepoInfo() - return root || undefined +export function getGitRepoRoot (startDir: string = process.cwd()): string | undefined { + let current = path.resolve(startDir) + for (;;) { + if (fsSync.existsSync(path.join(current, '.git'))) { + return current + } + const parent = path.dirname(current) + if (parent === current) { + return undefined + } + current = parent + } } export function getCiInformation (): CiInformation {