feat(cli): add early diagnostics to Construct [RED-641]

.github/workflows/test.yml

@@ -37,7 +37,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        os: [ubuntu-latest, windows-latest-x64]
+        os: [ubuntu-latest, blacksmith-4vcpu-windows-2025]
     name: test - ${{ matrix.os }}
     runs-on: ${{ matrix.os }}
     steps:
@@ -66,7 +66,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        os: [ubuntu-latest, windows-latest-x64]
+        os: [ubuntu-latest, blacksmith-4vcpu-windows-2025]
     name: e2e - checkly - ${{ matrix.os }}
     runs-on: ${{ matrix.os }}
     steps:
@@ -93,7 +93,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        os: [ubuntu-latest, windows-latest-x64]
+        os: [ubuntu-latest, blacksmith-4vcpu-windows-2025]
     name: e2e - create-checkly - ${{ matrix.os }}
     runs-on: ${{ matrix.os }}
     steps:

packages/cli/e2e/__tests__/test.spec.ts

@@ -174,8 +174,10 @@ describe('test', { timeout: 45000 }, () => {
         await runTest(fixt, [])
       } catch (err) {
         if (err instanceof ExecaError) {
-          expect((err.stderr as unknown as string).replace(/(\n {4})/gm, ''))
-            .toContain('Error: Resource of type \'check-group\' with logical id \'my-check-group\' already exists.')
+          // A duplicate logicalId is now reported as a fatal validation
+          // diagnostic (printed to stdout) rather than a thrown error.
+          expect((err.stdout as unknown as string).replace(/(\n {4})/gm, ''))
+            .toContain('A check-group with logicalId "my-check-group" already exists.')
           expect(err.exitCode).toBe(1)
         } else {
           throw err

packages/cli/src/constructs/__tests__/alert-channel.spec.ts

@@ -19,19 +19,24 @@ class TestAlertChannel extends AlertChannel {
 }
 
 describe('AlertChannel', () => {
-  it('should throw if the same logicalId is used twice', () => {
-    Session.project = new Project('project-id', {
+  it('should produce a diagnostic if the same logicalId is used twice', async () => {
+    const project = new Project('project-id', {
       name: 'Test Project',
       repoUrl: 'https://github.com/checkly/checkly-cli',
     })
+    Session.project = project
 
-    const add = () => {
-      new TestAlertChannel('foo', {
-      })
-    }
+    new TestAlertChannel('foo', {})
+    new TestAlertChannel('foo', {})
 
-    expect(add).not.toThrow()
-    expect(add).toThrow('already exists')
+    const diagnostics = new Diagnostics()
+    await project.validate(diagnostics)
+    expect(diagnostics.isFatal()).toBe(true)
+    expect(diagnostics.observations).toEqual(expect.arrayContaining([
+      expect.objectContaining({
+        message: expect.stringContaining('already exists'),
+      }),
+    ]))
   })
 
   it('should not throw if the same fromId() is used twice', () => {

packages/cli/src/constructs/__tests__/check-group.spec.ts

@@ -11,20 +11,28 @@ describe('CheckGroup', () => {
       expect(CheckGroupV1).toBe(CheckGroup)
     })
 
-    it('should throw if the same logicalId is used twice', () => {
-      Session.project = new Project('project-id', {
+    it('should produce a diagnostic if the same logicalId is used twice', async () => {
+      const project = new Project('project-id', {
         name: 'Test Project',
         repoUrl: 'https://github.com/checkly/checkly-cli',
       })
+      Session.project = project
 
-      const add = () => {
-        new CheckGroupV1('foo', {
-          name: 'Test',
-        })
-      }
+      new CheckGroupV1('foo', {
+        name: 'Test',
+      })
+      new CheckGroupV1('foo', {
+        name: 'Test',
+      })
 
-      expect(add).not.toThrow()
-      expect(add).toThrow('already exists')
+      const diagnostics = new Diagnostics()
+      await project.validate(diagnostics)
+      expect(diagnostics.isFatal()).toBe(true)
+      expect(diagnostics.observations).toEqual(expect.arrayContaining([
+        expect.objectContaining({
+          message: expect.stringContaining('already exists'),
+        }),
+      ]))
     })
 
     it('should not throw if the same fromId() is used twice', () => {
@@ -84,20 +92,28 @@ describe('CheckGroup', () => {
   })
 
   describe('v2', () => {
-    it('should throw if the same logicalId is used twice', () => {
-      Session.project = new Project('project-id', {
+    it('should produce a diagnostic if the same logicalId is used twice', async () => {
+      const project = new Project('project-id', {
         name: 'Test Project',
         repoUrl: 'https://github.com/checkly/checkly-cli',
       })
+      Session.project = project
 
-      const add = () => {
-        new CheckGroupV2('foo', {
-          name: 'Test',
-        })
-      }
+      new CheckGroupV2('foo', {
+        name: 'Test',
+      })
+      new CheckGroupV2('foo', {
+        name: 'Test',
+      })
 
-      expect(add).not.toThrow()
-      expect(add).toThrow('already exists')
+      const diagnostics = new Diagnostics()
+      await project.validate(diagnostics)
+      expect(diagnostics.isFatal()).toBe(true)
+      expect(diagnostics.observations).toEqual(expect.arrayContaining([
+        expect.objectContaining({
+          message: expect.stringContaining('already exists'),
+        }),
+      ]))
     })
 
     it('should not throw if the same fromId() is used twice', () => {

packages/cli/src/constructs/__tests__/early-diagnostics.spec.ts

@@ -0,0 +1,77 @@
+import { describe, it, expect, beforeEach } from 'vitest'
+
+import { Session } from '../session.js'
+import { Construct } from '../construct.js'
+import { Diagnostics, ErrorDiagnostic } from '../diagnostics.js'
+import { ConstructDiagnostics } from '../construct-diagnostics.js'
+
+class EarlyConstruct extends Construct {
+  constructor (logicalId: string) {
+    super('test', logicalId)
+    this.earlyDiagnostics.add(new ErrorDiagnostic({
+      title: 'Early problem',
+      message: 'Something the constructor noticed.',
+      error: new Error('Something the constructor noticed.'),
+    }))
+  }
+
+  describe (): string {
+    return `Test:${this.logicalId}`
+  }
+
+  synthesize () {
+    return null
+  }
+}
+
+describe('Construct early diagnostics', () => {
+  beforeEach(() => {
+    Session.reset()
+    Session.project = { addResource: () => {} } as any
+  })
+
+  it('surfaces diagnostics recorded in the constructor via validate()', async () => {
+    const construct = new EarlyConstruct('my-check')
+    const diagnostics = new Diagnostics()
+    await construct.validate(diagnostics)
+    expect(diagnostics.isFatal()).toBe(true)
+    expect(diagnostics.observations).toEqual(expect.arrayContaining([
+      expect.objectContaining({
+        message: expect.stringContaining('Something the constructor noticed.'),
+      }),
+    ]))
+  })
+
+  it('attributes early diagnostics to the construct when validated via ConstructDiagnostics', async () => {
+    const construct = new EarlyConstruct('my-check')
+    const diagnostics = new ConstructDiagnostics(construct)
+    await construct.validate(diagnostics)
+    expect(diagnostics.observations).toEqual(expect.arrayContaining([
+      expect.objectContaining({
+        title: expect.stringContaining('[Test:my-check]'),
+      }),
+    ]))
+  })
+
+  it('does not record any diagnostics when the constructor adds none', async () => {
+    class QuietConstruct extends Construct {
+      constructor (logicalId: string) {
+        super('test', logicalId)
+      }
+
+      describe (): string {
+        return `Quiet:${this.logicalId}`
+      }
+
+      synthesize () {
+        return null
+      }
+    }
+
+    const construct = new QuietConstruct('my-check')
+    const diagnostics = new Diagnostics()
+    await construct.validate(diagnostics)
+    expect(diagnostics.isFatal()).toBe(false)
+    expect(diagnostics.observations).toHaveLength(0)
+  })
+})

packages/cli/src/constructs/__tests__/private-location.spec.ts

@@ -5,21 +5,30 @@ import { Project } from '../project.js'
 import { Session } from '../session.js'
 
 describe('PrivateLocation', () => {
-  it('should throw if the same logicalId is used twice', () => {
-    Session.project = new Project('project-id', {
+  it('should produce a diagnostic if the same logicalId is used twice', async () => {
+    const project = new Project('project-id', {
       name: 'Test Project',
       repoUrl: 'https://github.com/checkly/checkly-cli',
     })
+    Session.project = project
 
-    const add = () => {
-      new PrivateLocation('foo', {
-        name: 'Test',
-        slugName: 'test',
-      })
-    }
+    new PrivateLocation('foo', {
+      name: 'Test',
+      slugName: 'test',
+    })
+    new PrivateLocation('foo', {
+      name: 'Test',
+      slugName: 'test',
+    })
 
-    expect(add).not.toThrow()
-    expect(add).toThrow('already exists')
+    const diagnostics = new Diagnostics()
+    await project.validate(diagnostics)
+    expect(diagnostics.isFatal()).toBe(true)
+    expect(diagnostics.observations).toEqual(expect.arrayContaining([
+      expect.objectContaining({
+        message: expect.stringContaining('already exists'),
+      }),
+    ]))
   })
 
   it('should not throw if the same fromId() is used twice', () => {

packages/cli/src/constructs/__tests__/project.spec.ts

@@ -1,11 +1,12 @@
 import { describe, it, expect, beforeEach } from 'vitest'
 
+import { Project } from '../project.js'
 import { Session } from '../session.js'
 import { Construct } from '../construct.js'
 import { Diagnostics } from '../diagnostics.js'
 
 class TestConstruct extends Construct {
-  constructor (logicalId: string) {
+  constructor (logicalId: any) {
     super('test', logicalId)
   }
 
@@ -74,4 +75,36 @@ describe('Construct logicalId validation', () => {
       }),
     ]))
   })
+
+  it('should produce a diagnostic for a non-string logicalId instead of throwing', async () => {
+    const construct = new TestConstruct(123)
+    expect(construct.logicalId).toBe('123')
+    const diagnostics = new Diagnostics()
+    await construct.validate(diagnostics)
+    expect(diagnostics.isFatal()).toBe(true)
+    expect(diagnostics.observations).toEqual(expect.arrayContaining([
+      expect.objectContaining({
+        message: expect.stringContaining('Expected a string but received type "number"'),
+      }),
+    ]))
+  })
+
+  it('should produce a diagnostic for a duplicate logicalId instead of throwing', async () => {
+    Session.reset()
+    const project = new Project('test-project', { name: 'Test' })
+    Session.project = project
+
+    project.addResource('check', 'duplicate-id', new TestConstruct('duplicate-id'))
+    project.addResource('check', 'duplicate-id', new TestConstruct('duplicate-id'))
+
+    const diagnostics = new Diagnostics()
+    await project.validate(diagnostics)
+    expect(diagnostics.isFatal()).toBe(true)
+    expect(diagnostics.observations).toEqual(expect.arrayContaining([
+      expect.objectContaining({
+        title: expect.stringContaining('[Test:duplicate-id]'),
+        message: expect.stringContaining('A check with logicalId "duplicate-id" already exists'),
+      }),
+    ]))
+  })
 })

packages/cli/src/constructs/__tests__/status-page-service.spec.ts

@@ -5,20 +5,28 @@ import { Project } from '../project.js'
 import { Session } from '../session.js'
 
 describe('StatusPageService', () => {
-  it('should throw if the same logicalId is used twice', () => {
-    Session.project = new Project('project-id', {
+  it('should produce a diagnostic if the same logicalId is used twice', async () => {
+    const project = new Project('project-id', {
       name: 'Test Project',
       repoUrl: 'https://github.com/checkly/checkly-cli',
     })
+    Session.project = project
 
-    const add = () => {
-      new StatusPageService('foo', {
-        name: 'foo',
-      })
-    }
+    new StatusPageService('foo', {
+      name: 'foo',
+    })
+    new StatusPageService('foo', {
+      name: 'foo',
+    })
 
-    expect(add).not.toThrow()
-    expect(add).toThrow('already exists')
+    const diagnostics = new Diagnostics()
+    await project.validate(diagnostics)
+    expect(diagnostics.isFatal()).toBe(true)
+    expect(diagnostics.observations).toEqual(expect.arrayContaining([
+      expect.objectContaining({
+        message: expect.stringContaining('already exists'),
+      }),
+    ]))
   })
 
   it('should not throw if the same fromId() is used twice', () => {

packages/cli/src/constructs/construct.ts

@@ -57,6 +57,13 @@ export abstract class Construct implements Validate, Bundle {
   member: boolean
   /** Absolute path to the check file that created this construct */
   checkFileAbsolutePath?: string
+  /**
+   * Diagnostics recorded during construction. A constructor cannot perform the
+   * async work that validate() can, so any issue it notices (e.g. an argument
+   * of the wrong type) can be added here and is merged into the caller's
+   * Diagnostics by validate().
+   */
+  readonly earlyDiagnostics = new Diagnostics()
 
   /**
    * Creates a new construct instance.
@@ -67,6 +74,13 @@ export abstract class Construct implements Validate, Bundle {
    * @param member Whether this construct is a member of the project
    */
   constructor (type: string, logicalId: string, physicalId?: string | number, member?: boolean) {
+    if (typeof logicalId !== 'string') {
+      this.earlyDiagnostics.add(new InvalidPropertyValueDiagnostic(
+        'logicalId',
+        new Error(`Expected a string but received type "${typeof logicalId}".`),
+      ))
+      logicalId = String(logicalId)
+    }
     this.logicalId = logicalId
     this.type = type
     this.physicalId = physicalId
@@ -128,6 +142,8 @@ export abstract class Construct implements Validate, Bundle {
    */
   // eslint-disable-next-line require-await
   async validate (diagnostics: Diagnostics): Promise<void> {
+    diagnostics.extend(this.earlyDiagnostics)
+
     if (!LOGICAL_ID_PATTERN.test(this.logicalId)) {
       diagnostics.add(new InvalidPropertyValueDiagnostic(
         'logicalId',