From de217d37769b216d1690a04915c63c058bc7facf Mon Sep 17 00:00:00 2001 From: Daniel Paulus Date: Wed, 15 Jul 2026 13:30:01 +0200 Subject: [PATCH] fix(constructs): emit numeric gRPC health-check status from healthCheckStatus() (SIM-307) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GrpcAssertionBuilder.healthCheckStatus() returned a GeneralAssertionBuilder, so the CLI's own documented example .equals('SERVING') shipped target "SERVING" to the API. The go-runner evaluates gRPC health status numerically (strconv.Atoi), so the label could never pass ("Invalid. Target health status is not a number") — the advertised example built a permanently-failing check. Only the webapp normalized the label to a number. - healthCheckStatus() now returns a dedicated HealthCheckStatusAssertionBuilder exposing only .equals()/.notEquals() (the only comparisons the runner supports), accepting a GrpcHealthStatus label or the raw 0|1|2|3 enum and emitting the numeric wire target. Mirrors the webapp's grpcHealthStatusTargetByLabel mapping (single source of truth). - Codegen reverse-maps the number back to the label so an imported check round-trips to .equals('SERVING'); unmappable/legacy targets fall back to a raw assertion object literal so the generated code always type-checks. - Validation flags a GRPC_HEALTHCHECK_STATUS target that isn't 0-3, catching object-literal callers who bypass the builder. Reviewed via tri-model daniel-review (Codex + Gemini + Claude); all findings fixed. Build clean, 21 gRPC unit/codegen tests green. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_013AYwkvoaANQTH5dGStbBj2 --- .../__tests__/grpc-assertion-codegen.spec.ts | 61 ++++++++++ .../constructs/__tests__/grpc-monitor.spec.ts | 113 ++++++++++++++++++ .../src/constructs/grpc-assertion-codegen.ts | 72 ++++++++++- .../constructs/grpc-assertion-validation.ts | 22 +++- packages/cli/src/constructs/grpc-assertion.ts | 65 +++++++++- 5 files changed, 323 insertions(+), 10 deletions(-) create mode 100644 packages/cli/src/constructs/__tests__/grpc-assertion-codegen.spec.ts diff --git a/packages/cli/src/constructs/__tests__/grpc-assertion-codegen.spec.ts b/packages/cli/src/constructs/__tests__/grpc-assertion-codegen.spec.ts new file mode 100644 index 000000000..66edb1d69 --- /dev/null +++ b/packages/cli/src/constructs/__tests__/grpc-assertion-codegen.spec.ts @@ -0,0 +1,61 @@ +import { describe, it, expect } from 'vitest' + +import { valueForGrpcAssertion } from '../grpc-assertion-codegen.js' +import { GrpcAssertion } from '../grpc-assertion.js' +import { GeneratedFile, Output } from '../../sourcegen/index.js' + +function render (assertion: GrpcAssertion): string { + const output = new Output() + const file = new GeneratedFile('foo.ts') + const result = valueForGrpcAssertion(file, assertion) + result.render(output) + return output.finalize() +} + +describe('gRPC Assertion Codegen', () => { + it('round-trips the numeric health-check status target back to its label', () => { + const cases: { input: GrpcAssertion, expected: string }[] = [ + { + input: { source: 'GRPC_HEALTHCHECK_STATUS', property: '', comparison: 'EQUALS', target: '0', regex: null }, + expected: 'GrpcAssertionBuilder.healthCheckStatus().equals(\'UNKNOWN\')\n', + }, + { + input: { source: 'GRPC_HEALTHCHECK_STATUS', property: '', comparison: 'EQUALS', target: '1', regex: null }, + expected: 'GrpcAssertionBuilder.healthCheckStatus().equals(\'SERVING\')\n', + }, + { + input: { source: 'GRPC_HEALTHCHECK_STATUS', property: '', comparison: 'EQUALS', target: '2', regex: null }, + expected: 'GrpcAssertionBuilder.healthCheckStatus().equals(\'NOT_SERVING\')\n', + }, + { + input: { source: 'GRPC_HEALTHCHECK_STATUS', property: '', comparison: 'EQUALS', target: '3', regex: null }, + expected: 'GrpcAssertionBuilder.healthCheckStatus().equals(\'SERVICE_UNKNOWN\')\n', + }, + { + input: { source: 'GRPC_HEALTHCHECK_STATUS', property: '', comparison: 'NOT_EQUALS', target: '2', regex: null }, + expected: 'GrpcAssertionBuilder.healthCheckStatus().notEquals(\'NOT_SERVING\')\n', + }, + ] + for (const test of cases) { + expect(render(test.input)).toEqual(test.expected) + } + }) + + it('falls back to a raw assertion object literal for an unmappable health-check status target', () => { + // '9' is out of the 0-3 serving-status range and isn't a valid label, so a + // builder call would not type-check against the narrowed `equals`/`notEquals` + // parameter type. The raw `Assertion` shape compiles (target is a plain string) + // and faithfully round-trips the stored value instead. + const input: GrpcAssertion = + { source: 'GRPC_HEALTHCHECK_STATUS', property: '', comparison: 'EQUALS', target: '9', regex: null } + expect(render(input)).toEqual( + '{\n' + + ' source: \'GRPC_HEALTHCHECK_STATUS\',\n' + + ' comparison: \'EQUALS\',\n' + + ' target: \'9\',\n' + + ' property: \'\',\n' + + ' regex: null,\n' + + '}\n', + ) + }) +}) diff --git a/packages/cli/src/constructs/__tests__/grpc-monitor.spec.ts b/packages/cli/src/constructs/__tests__/grpc-monitor.spec.ts index 782d5484a..b70479168 100644 --- a/packages/cli/src/constructs/__tests__/grpc-monitor.spec.ts +++ b/packages/cli/src/constructs/__tests__/grpc-monitor.spec.ts @@ -108,6 +108,58 @@ describe('GrpcMonitor', () => { expect(bundle.synthesize()).toMatchObject({ groupId: { ref: 'main-group' } }) }) + describe('healthCheckStatus assertion builder', () => { + it('maps each label to its numeric serving-status wire target', () => { + const cases: { label: 'UNKNOWN' | 'SERVING' | 'NOT_SERVING' | 'SERVICE_UNKNOWN', target: string }[] = [ + { label: 'UNKNOWN', target: '0' }, + { label: 'SERVING', target: '1' }, + { label: 'NOT_SERVING', target: '2' }, + { label: 'SERVICE_UNKNOWN', target: '3' }, + ] + for (const { label, target } of cases) { + expect(GrpcAssertionBuilder.healthCheckStatus().equals(label)).toEqual({ + source: 'GRPC_HEALTHCHECK_STATUS', + comparison: 'EQUALS', + target, + property: '', + regex: null, + }) + } + }) + + it('emits notEquals with the numeric target', () => { + expect(GrpcAssertionBuilder.healthCheckStatus().notEquals('NOT_SERVING')).toEqual({ + source: 'GRPC_HEALTHCHECK_STATUS', + comparison: 'NOT_EQUALS', + target: '2', + property: '', + regex: null, + }) + }) + + it('accepts the raw numeric enum value for power users', () => { + expect(GrpcAssertionBuilder.healthCheckStatus().equals(1)).toEqual({ + source: 'GRPC_HEALTHCHECK_STATUS', + comparison: 'EQUALS', + target: '1', + property: '', + regex: null, + }) + }) + + it('accepts the falsy raw numeric enum value 0 (UNKNOWN)', () => { + // Pins the falsy boundary: `0` must not be treated as "no target given" by + // grpcHealthStatusWireTarget's `typeof target === 'number'` branch. + expect(GrpcAssertionBuilder.healthCheckStatus().equals(0)).toEqual({ + source: 'GRPC_HEALTHCHECK_STATUS', + comparison: 'EQUALS', + target: '0', + property: '', + regex: null, + }) + }) + }) + describe('validation', () => { it('should error if degradedResponseTime is above 180000', async () => { setupProject() @@ -236,6 +288,67 @@ describe('GrpcMonitor', () => { expect(diags.isFatal()).toEqual(false) }) + it('should error on a health-check status assertion with a non-numeric target', async () => { + setupProject() + const check = new GrpcMonitor('test-check', { + name: 'Test Check', + request: { + ...request, + // Object literal bypasses the builder: a raw label can never pass the + // runner's numeric evaluation. + assertions: [ + { source: 'GRPC_HEALTHCHECK_STATUS', property: '', comparison: 'EQUALS', target: 'SERVING', regex: null }, + ], + }, + }) + const diags = new Diagnostics() + await check.validate(diags) + expect(diags.isFatal()).toEqual(true) + expect(diags.observations).toEqual(expect.arrayContaining([ + expect.objectContaining({ + message: expect.stringContaining('has an invalid health-check status target "SERVING"'), + }), + ])) + }) + + it('should error on a health-check status assertion with an out-of-range numeric target', async () => { + setupProject() + const check = new GrpcMonitor('test-check', { + name: 'Test Check', + request: { + ...request, + assertions: [ + { source: 'GRPC_HEALTHCHECK_STATUS', property: '', comparison: 'EQUALS', target: '9', regex: null }, + ], + }, + }) + const diags = new Diagnostics() + await check.validate(diags) + expect(diags.isFatal()).toEqual(true) + expect(diags.observations).toEqual(expect.arrayContaining([ + expect.objectContaining({ + message: expect.stringContaining('has an invalid health-check status target "9"'), + }), + ])) + }) + + it('should not error on a health-check status assertion with a numeric target', async () => { + setupProject() + const check = new GrpcMonitor('test-check', { + name: 'Test Check', + request: { + ...request, + assertions: [ + { source: 'GRPC_HEALTHCHECK_STATUS', property: '', comparison: 'EQUALS', target: '1', regex: null }, + GrpcAssertionBuilder.healthCheckStatus().equals('SERVING'), + ], + }, + }) + const diags = new Diagnostics() + await check.validate(diags) + expect(diags.isFatal()).toEqual(false) + }) + it('should not error on a property carried by any gRPC source', async () => { setupProject() const check = new GrpcMonitor('test-check', { diff --git a/packages/cli/src/constructs/grpc-assertion-codegen.ts b/packages/cli/src/constructs/grpc-assertion-codegen.ts index ef0885c67..72ac5585a 100644 --- a/packages/cli/src/constructs/grpc-assertion-codegen.ts +++ b/packages/cli/src/constructs/grpc-assertion-codegen.ts @@ -1,6 +1,69 @@ -import { GeneratedFile, Value } from '../sourcegen/index.js' +import { expr, ident, object, GeneratedFile, Value } from '../sourcegen/index.js' import { unsupportedAssertionSource, valueForGeneralAssertion, valueForNumericAssertion } from './internal/assertion-codegen.js' -import { GrpcAssertion } from './grpc-assertion.js' +import { GrpcAssertion, GrpcHealthStatus, grpcHealthStatusTargetByLabel } from './grpc-assertion.js' + +// Reverse of grpcHealthStatusTargetByLabel: numeric wire target -> friendly label. +// Derived from the shared map so the label↔number mapping lives in one place. +const grpcHealthStatusLabelByTarget: Record = Object.fromEntries( + Object.entries(grpcHealthStatusTargetByLabel).map(([label, target]) => [target, label as GrpcHealthStatus]), +) + +function isGrpcHealthStatusLabel (value: string): value is GrpcHealthStatus { + return Object.hasOwn(grpcHealthStatusTargetByLabel, value) +} + +// Resolves a stored target to the label the builder's (narrowed) `equals`/`notEquals` +// parameter type accepts — either the label for a known number, or the target itself +// when it is already a valid label (a legacy/hand-written check may store one). +// Returns undefined for anything else (out-of-range number, empty, garbage), since a +// builder call for those would not compile against the narrowed builder type. +function grpcHealthStatusLabelForTarget (target: string): GrpcHealthStatus | undefined { + return grpcHealthStatusLabelByTarget[target] ?? (isGrpcHealthStatusLabel(target) ? target : undefined) +} + +// Emits `GrpcAssertionBuilder.healthCheckStatus().equals('SERVING')`, reverse-mapping +// the numeric wire target back to its label. When the stored target maps to neither +// a known number nor a valid label (out-of-range, empty, or otherwise unmappable), a +// builder call would not type-check against the narrowed builder parameter type, so +// the raw `Assertion` object-literal shape is emitted instead — which the `Assertion` +// type accepts as a plain string and faithfully round-trips the stored value. +function valueForGrpcHealthCheckStatusAssertion (assertion: GrpcAssertion): Value { + const label = grpcHealthStatusLabelForTarget(assertion.target) + if (label === undefined) { + return object(builder => { + builder.string('source', assertion.source) + builder.string('comparison', assertion.comparison) + builder.string('target', assertion.target) + builder.string('property', assertion.property) + if (assertion.regex === null) { + builder.null('regex') + } else { + builder.string('regex', assertion.regex) + } + }) + } + + return expr(ident('GrpcAssertionBuilder'), builder => { + builder.member(ident('healthCheckStatus')) + builder.call(() => {}) + switch (assertion.comparison) { + case 'EQUALS': + builder.member(ident('equals')) + builder.call(builder => { + builder.string(label) + }) + break + case 'NOT_EQUALS': + builder.member(ident('notEquals')) + builder.call(builder => { + builder.string(label) + }) + break + default: + throw new Error(`Unsupported comparison ${assertion.comparison} for assertion source ${assertion.source}`) + } + }) +} export function valueForGrpcAssertion (genfile: GeneratedFile, assertion: GrpcAssertion): Value { genfile.namedImport('GrpcAssertionBuilder', 'checkly/constructs') @@ -11,10 +74,7 @@ export function valueForGrpcAssertion (genfile: GeneratedFile, assertion: GrpcAs case 'GRPC_STATUS_CODE': return valueForNumericAssertion('GrpcAssertionBuilder', 'statusCode', assertion) case 'GRPC_HEALTHCHECK_STATUS': - return valueForGeneralAssertion('GrpcAssertionBuilder', 'healthCheckStatus', assertion, { - hasProperty: false, - hasRegex: false, - }) + return valueForGrpcHealthCheckStatusAssertion(assertion) case 'GRPC_RESPONSE': return valueForGeneralAssertion('GrpcAssertionBuilder', 'responseMessage', assertion, { hasProperty: true, diff --git a/packages/cli/src/constructs/grpc-assertion-validation.ts b/packages/cli/src/constructs/grpc-assertion-validation.ts index 1a4f1b601..523ecd924 100644 --- a/packages/cli/src/constructs/grpc-assertion-validation.ts +++ b/packages/cli/src/constructs/grpc-assertion-validation.ts @@ -1,7 +1,15 @@ import { Diagnostics } from './diagnostics.js' -import { GrpcAssertion } from './grpc-assertion.js' +import { GrpcAssertion, grpcHealthStatusTargetByLabel } from './grpc-assertion.js' import { addAssertionDiagnostic, quotedKeys } from './internal/assertion-validation.js' +// The gRPC runner evaluates health-check status numerically (it parses `target` +// with `Atoi`), so the only valid wire targets are the numeric serving-status +// values. A label like "SERVING" — or garbage — sent as a raw target can never +// pass. The GrpcAssertionBuilder emits these numbers; object-literal users bypass it. +const grpcHealthStatusTargets: Record = Object.fromEntries( + Object.values(grpcHealthStatusTargetByLabel).map(target => [target, true]), +) + // Keyed by the source union so a member added to GrpcAssertion['source'] without a // matching entry here is a compile-time error. const assertionSources: Record = { @@ -62,4 +70,16 @@ export function validateGrpcAssertion ( + `${assertion.comparison === '' ? '(none)' : `"${assertion.comparison}"`}. ` + `Expected one of ${quotedKeys(assertionComparisons)}.`) } + + // The runner evaluates health-check status numerically, so a non-numeric target + // (e.g. the label "SERVING", written directly on an object literal) can never + // pass. Use GrpcAssertionBuilder.healthCheckStatus() to emit the numeric value. + if (assertion.source === 'GRPC_HEALTHCHECK_STATUS' + && !Object.hasOwn(grpcHealthStatusTargets, assertion.target)) { + addAssertionDiagnostic(diagnostics, + `The assertion at "${location}" has an invalid health-check status target ` + + `${assertion.target === '' ? '(none)' : `"${assertion.target}"`}. ` + + `Expected a numeric serving-status value: one of ${quotedKeys(grpcHealthStatusTargets)}. ` + + `Use GrpcAssertionBuilder.healthCheckStatus().equals('SERVING') to emit it from a label.`) + } } diff --git a/packages/cli/src/constructs/grpc-assertion.ts b/packages/cli/src/constructs/grpc-assertion.ts index 999d7b04f..08cfaf976 100644 --- a/packages/cli/src/constructs/grpc-assertion.ts +++ b/packages/cli/src/constructs/grpc-assertion.ts @@ -1,4 +1,4 @@ -import { Assertion as CoreAssertion, NumericAssertionBuilder, GeneralAssertionBuilder } from './internal/assertion.js' +import { Assertion as CoreAssertion, NumericAssertionBuilder, GeneralAssertionBuilder, toAssertion } from './internal/assertion.js' type GrpcAssertionSource = | 'RESPONSE_TIME' @@ -10,6 +10,62 @@ type GrpcAssertionSource = export type GrpcAssertion = CoreAssertion +/** + * Health-check serving-status labels for use with + * {@link GrpcAssertionBuilder.healthCheckStatus}. These are the labels of the + * `grpc.health.v1.HealthCheckResponse.ServingStatus` enum. + * + * @example + * ```typescript + * GrpcAssertionBuilder.healthCheckStatus().equals('SERVING') + * ``` + */ +export type GrpcHealthStatus = 'UNKNOWN' | 'SERVING' | 'NOT_SERVING' | 'SERVICE_UNKNOWN' + +/** + * Maps each {@link GrpcHealthStatus} label to the numeric wire target the gRPC + * runner evaluates health-check status against (it parses `target` with `Atoi`). + * These are the `grpc.health.v1.HealthCheckResponse.ServingStatus` enum values: + * 0 = UNKNOWN, 1 = SERVING, 2 = NOT_SERVING, 3 = SERVICE_UNKNOWN. + * + * This is the single source of truth for the label↔number mapping — the codegen + * reverse-maps through it rather than duplicating the literals. + */ +export const grpcHealthStatusTargetByLabel: Record = { + UNKNOWN: '0', + SERVING: '1', + NOT_SERVING: '2', + SERVICE_UNKNOWN: '3', +} + +// A health-check status target: either a friendly label or, for power users, the +// raw numeric enum value the wire actually carries. +type GrpcHealthStatusTarget = GrpcHealthStatus | 0 | 1 | 2 | 3 + +function grpcHealthStatusWireTarget (target: GrpcHealthStatusTarget): string { + // Types keep TS callers to a known label or number, but a JS caller can bypass + // them with an invalid string (e.g. lowercase 'serving'). Falling back to the + // raw value — mirroring the webapp's `?? assertion.target` — preserves it so it + // still surfaces (rather than as an empty string) in the validation diagnostic. + return typeof target === 'number' ? target.toString() : grpcHealthStatusTargetByLabel[target] ?? String(target) +} + +/** + * Assertion builder for the gRPC health-check serving status. The runner evaluates + * the status numerically, so `.equals()`/`.notEquals()` accept a friendly label + * (or the raw enum number) and emit the numeric wire target. Only EQUALS and + * NOT_EQUALS are supported — the runner rejects other comparisons for this source. + */ +class HealthCheckStatusAssertionBuilder { + equals (target: GrpcHealthStatusTarget): GrpcAssertion { + return toAssertion('GRPC_HEALTHCHECK_STATUS', 'EQUALS', grpcHealthStatusWireTarget(target)) + } + + notEquals (target: GrpcHealthStatusTarget): GrpcAssertion { + return toAssertion('GRPC_HEALTHCHECK_STATUS', 'NOT_EQUALS', grpcHealthStatusWireTarget(target)) + } +} + /** * Builder class for creating gRPC monitor assertions. * Provides methods to create assertions for gRPC call responses. @@ -51,10 +107,13 @@ export class GrpcAssertionBuilder { /** * Creates an assertion builder for the gRPC health-check status (HEALTH mode). - * @returns A general assertion builder for the health-check status. + * `.equals()`/`.notEquals()` accept a friendly {@link GrpcHealthStatus} label + * (e.g. `'SERVING'`) — or the raw enum number `0`–`3` — and emit the numeric + * serving-status value the runner evaluates. + * @returns An assertion builder for the health-check status. */ static healthCheckStatus () { - return new GeneralAssertionBuilder('GRPC_HEALTHCHECK_STATUS') + return new HealthCheckStatusAssertionBuilder() } /**