From a7f581bde557f4c288f042843ed0b71458748e4a Mon Sep 17 00:00:00 2001 From: Angel Kou Date: Thu, 5 Mar 2026 17:59:56 -0800 Subject: [PATCH] Add timezone to workflow and pass FF (#334) * Add timezone to workflow and pass FF * Prettier fixes * Prettier fixes * Prettier fixes * Guard timezone autocomplete behind FF * Prettier fix * Address PR comments * Prettier fix * Remove comma * Remove template assignment * Move description * Fix test * Prettier again! * Address comments * Change error when timezone key is entered but FF is off * Prettier --------- Co-authored-by: Angel Kou --- expressions/src/features.test.ts | 3 +- expressions/src/features.ts | 9 +- languageservice/src/complete.test.ts | 42 ++++++ languageservice/src/complete.ts | 5 + languageservice/src/hover.test.ts | 8 +- languageservice/src/validate.ts | 3 +- workflow-parser/src/model/convert.test.ts | 137 ++++++++++++++++++ workflow-parser/src/model/convert.ts | 17 ++- workflow-parser/src/model/converter/events.ts | 48 ++++-- .../src/model/workflow-template.ts | 1 + workflow-parser/src/workflow-v1.0.json | 15 +- 11 files changed, 267 insertions(+), 21 deletions(-) diff --git a/expressions/src/features.test.ts b/expressions/src/features.test.ts index f280c14..0ff1de7 100644 --- a/expressions/src/features.test.ts +++ b/expressions/src/features.test.ts @@ -54,7 +54,8 @@ describe("FeatureFlags", () => { expect(flags.getEnabledFeatures()).toEqual([ "missingInputsQuickfix", "blockScalarChompingWarning", - "allowCaseFunction" + "allowCaseFunction", + "allowCronTimezone" ]); }); }); diff --git a/expressions/src/features.ts b/expressions/src/features.ts index e142aac..163629c 100644 --- a/expressions/src/features.ts +++ b/expressions/src/features.ts @@ -34,6 +34,12 @@ export interface ExperimentalFeatures { * @default false */ allowCaseFunction?: boolean; + + /** + * Enable the timezone input in cron schedule mappings. + * @default false + */ + allowCronTimezone?: boolean; } /** @@ -48,7 +54,8 @@ export type ExperimentalFeatureKey = Exclude; const allFeatureKeys: ExperimentalFeatureKey[] = [ "missingInputsQuickfix", "blockScalarChompingWarning", - "allowCaseFunction" + "allowCaseFunction", + "allowCronTimezone" ]; export class FeatureFlags { diff --git a/languageservice/src/complete.test.ts b/languageservice/src/complete.test.ts index 99f86c0..aa7384b 100644 --- a/languageservice/src/complete.test.ts +++ b/languageservice/src/complete.test.ts @@ -925,3 +925,45 @@ jobs: }); }); }); + +describe("schedule timezone completion", () => { + it("includes timezone when allowCronTimezone is enabled", async () => { + const input = `on: + schedule: + - |`; + const result = await complete(...getPositionFromCursor(input), { + featureFlags: new FeatureFlags({allowCronTimezone: true}) + }); + + expect(result).not.toBeUndefined(); + const labels = result.map(x => x.label); + expect(labels).toContain("cron"); + expect(labels).toContain("timezone"); + }); + + it("excludes timezone when allowCronTimezone is disabled", async () => { + const input = `on: + schedule: + - |`; + const result = await complete(...getPositionFromCursor(input), { + featureFlags: new FeatureFlags({allowCronTimezone: false}) + }); + + expect(result).not.toBeUndefined(); + const labels = result.map(x => x.label); + expect(labels).toContain("cron"); + expect(labels).not.toContain("timezone"); + }); + + it("excludes timezone when no feature flags are provided", async () => { + const input = `on: + schedule: + - |`; + const result = await complete(...getPositionFromCursor(input)); + + expect(result).not.toBeUndefined(); + const labels = result.map(x => x.label); + expect(labels).toContain("cron"); + expect(labels).not.toContain("timezone"); + }); +}); diff --git a/languageservice/src/complete.ts b/languageservice/src/complete.ts index 32c2e5e..a861c75 100644 --- a/languageservice/src/complete.ts +++ b/languageservice/src/complete.ts @@ -163,6 +163,11 @@ export async function complete( values = filterActionRunsCompletions(values, path, parsedTemplate.value); } + // Filter `timezone` from schedule completions when the feature flag is disabled + if (!config?.featureFlags?.isEnabled("allowCronTimezone") && parent?.definition?.key === "schedule") { + values = values.filter(v => v.label !== "timezone"); + } + // Offer "(switch to list)" / "(switch to mapping)" when the schema allows alternative forms const escapeHatches = getEscapeHatchCompletions(token, keyToken, indentString, newPos, schema); values.push(...escapeHatches); diff --git a/languageservice/src/hover.test.ts b/languageservice/src/hover.test.ts index 03c2482..64eea45 100644 --- a/languageservice/src/hover.test.ts +++ b/languageservice/src/hover.test.ts @@ -120,7 +120,9 @@ jobs: `; const result = await hover(...getPositionFromCursor(input)); expect(result).not.toBeUndefined(); - expect(result?.contents).toEqual(""); + expect(result?.contents).toEqual( + "A cron expression that represents a schedule. A scheduled workflow will run at most once every 5 minutes." + ); }); it("on an invalid cron schedule", async () => { @@ -130,7 +132,9 @@ jobs: `; const result = await hover(...getPositionFromCursor(input)); expect(result).not.toBeUndefined(); - expect(result?.contents).toEqual(""); + expect(result?.contents).toEqual( + "A cron expression that represents a schedule. A scheduled workflow will run at most once every 5 minutes." + ); }); it("shows context inherited from parent nodes", async () => { diff --git a/languageservice/src/validate.ts b/languageservice/src/validate.ts index 43b253e..a0c0bd8 100644 --- a/languageservice/src/validate.ts +++ b/languageservice/src/validate.ts @@ -84,7 +84,8 @@ async function validateWorkflow(textDocument: TextDocument, config?: ValidationC // Errors will be updated in the context. Attempt to do the conversion anyway in order to give the user more information const template = await getOrConvertWorkflowTemplate(result.context, result.value, textDocument.uri, config, { fetchReusableWorkflowDepth: config?.fileProvider ? 1 : 0, - errorPolicy: ErrorPolicy.TryConversion + errorPolicy: ErrorPolicy.TryConversion, + featureFlags: config?.featureFlags }); // Validate expressions and value providers diff --git a/workflow-parser/src/model/convert.test.ts b/workflow-parser/src/model/convert.test.ts index 3d92f4a..03fd688 100644 --- a/workflow-parser/src/model/convert.test.ts +++ b/workflow-parser/src/model/convert.test.ts @@ -1,4 +1,5 @@ /* eslint-disable @typescript-eslint/no-non-null-assertion */ +import {FeatureFlags} from "@actions/expressions/features"; import {nullTrace} from "../test-utils/null-trace.js"; import {parseWorkflow} from "../workflows/workflow-parser.js"; import {convertWorkflowTemplate, ErrorPolicy} from "./convert.js"; @@ -578,4 +579,140 @@ jobs: } }); }); + + describe("schedule timezone with feature flags", () => { + it("allows timezone when allowCronTimezone is enabled", async () => { + const result = parseWorkflow( + { + name: "wf.yaml", + content: `on: + schedule: + - cron: '0 0 * * *' + timezone: America/New_York +jobs: + build: + runs-on: ubuntu-latest` + }, + nullTrace + ); + + const template = await convertWorkflowTemplate(result.context, result.value!, undefined, { + errorPolicy: ErrorPolicy.TryConversion, + featureFlags: new FeatureFlags({allowCronTimezone: true}) + }); + + expect(result.context.errors.getErrors()).toHaveLength(0); + expect(template.events?.schedule).toHaveLength(1); + expect(template.events?.schedule?.[0]).toEqual({ + cron: "0 0 * * *", + timezone: "America/New_York" + }); + }); + + it("reports error when timezone is present but allowCronTimezone is disabled", async () => { + const result = parseWorkflow( + { + name: "wf.yaml", + content: `on: + schedule: + - cron: '0 0 * * *' + timezone: America/New_York +jobs: + build: + runs-on: ubuntu-latest` + }, + nullTrace + ); + + const template = await convertWorkflowTemplate(result.context, result.value!, undefined, { + errorPolicy: ErrorPolicy.TryConversion, + featureFlags: new FeatureFlags({allowCronTimezone: false}) + }); + + // When timezone feature is disabled, error points at the timezone key + expect(result.context.errors.getErrors()).toHaveLength(1); + expect(result.context.errors.getErrors()[0].message).toContain("Key 'timezone' is not supported"); + // Schedule entry is dropped due to unsupported key + expect(template.events?.schedule).toHaveLength(0); + }); + + it("reports error when timezone is present with no feature flags provided", async () => { + const result = parseWorkflow( + { + name: "wf.yaml", + content: `on: + schedule: + - cron: '0 0 * * *' + timezone: America/New_York +jobs: + build: + runs-on: ubuntu-latest` + }, + nullTrace + ); + + await convertWorkflowTemplate(result.context, result.value!, undefined, { + errorPolicy: ErrorPolicy.TryConversion + }); + + // Default is timezone disabled, so error points at the timezone key + expect(result.context.errors.getErrors()).toHaveLength(1); + expect(result.context.errors.getErrors()[0].message).toContain("Key 'timezone' is not supported"); + }); + + it("reports error when cron is missing from schedule entry", async () => { + const result = parseWorkflow( + { + name: "wf.yaml", + content: `on: + schedule: + - timezone: America/New_York +jobs: + build: + runs-on: ubuntu-latest` + }, + nullTrace + ); + + const template = await convertWorkflowTemplate(result.context, result.value!, undefined, { + errorPolicy: ErrorPolicy.TryConversion, + featureFlags: new FeatureFlags({allowCronTimezone: true}) + }); + + // Both schema validation and converter report the missing cron + expect(result.context.errors.getErrors().length).toBeGreaterThanOrEqual(1); + const errorMessages = result.context.errors + .getErrors() + .map(e => e.message) + .join(", "); + expect(errorMessages).toMatch(/Required property is missing: cron|Missing required key 'cron'/); + expect(template.events?.schedule).toHaveLength(0); + }); + + it("converts schedule without timezone when allowCronTimezone is enabled", async () => { + const result = parseWorkflow( + { + name: "wf.yaml", + content: `on: + schedule: + - cron: '0 0 * * *' +jobs: + build: + runs-on: ubuntu-latest` + }, + nullTrace + ); + + const template = await convertWorkflowTemplate(result.context, result.value!, undefined, { + errorPolicy: ErrorPolicy.TryConversion, + featureFlags: new FeatureFlags({allowCronTimezone: true}) + }); + + expect(result.context.errors.getErrors()).toHaveLength(0); + expect(template.events?.schedule).toHaveLength(1); + expect(template.events?.schedule?.[0]).toEqual({ + cron: "0 0 * * *" + }); + }); + }); }); diff --git a/workflow-parser/src/model/convert.ts b/workflow-parser/src/model/convert.ts index afc14d4..ff2b38d 100644 --- a/workflow-parser/src/model/convert.ts +++ b/workflow-parser/src/model/convert.ts @@ -1,3 +1,4 @@ +import {FeatureFlags} from "@actions/expressions/features"; import {TemplateContext} from "../templates/template-context.js"; import {TemplateToken, TemplateTokenError} from "../templates/tokens/template-token.js"; import {FileProvider} from "../workflows/file-provider.js"; @@ -37,12 +38,18 @@ export type WorkflowTemplateConverterOptions = { * By default, conversion will be skipped if there are errors in the {@link TemplateContext}. */ errorPolicy?: ErrorPolicy; + + /** + * Optional feature flags to control which experimental features are enabled. + */ + featureFlags?: FeatureFlags; }; const defaultOptions: Required = { maxReusableWorkflowDepth: 4, fetchReusableWorkflowDepth: 0, - errorPolicy: ErrorPolicy.ReturnErrorsOnly + errorPolicy: ErrorPolicy.ReturnErrorsOnly, + featureFlags: new FeatureFlags() }; export async function convertWorkflowTemplate( @@ -54,6 +61,11 @@ export async function convertWorkflowTemplate( const result = {} as WorkflowTemplate; const opts = getOptionsWithDefaults(options); + // Store feature flags in context state so converters can access them + if (opts.featureFlags) { + context.state["featureFlags"] = opts.featureFlags; + } + if (context.errors.getErrors().length > 0 && opts.errorPolicy === ErrorPolicy.ReturnErrorsOnly) { result.errors = context.errors.getErrors().map(x => ({ Message: x.message @@ -142,6 +154,7 @@ function getOptionsWithDefaults(options: WorkflowTemplateConverterOptions): Requ options.fetchReusableWorkflowDepth !== undefined ? options.fetchReusableWorkflowDepth : defaultOptions.fetchReusableWorkflowDepth, - errorPolicy: options.errorPolicy !== undefined ? options.errorPolicy : defaultOptions.errorPolicy + errorPolicy: options.errorPolicy !== undefined ? options.errorPolicy : defaultOptions.errorPolicy, + featureFlags: options.featureFlags ?? defaultOptions.featureFlags }; } diff --git a/workflow-parser/src/model/converter/events.ts b/workflow-parser/src/model/converter/events.ts index 965d4d1..28f5c1d 100644 --- a/workflow-parser/src/model/converter/events.ts +++ b/workflow-parser/src/model/converter/events.ts @@ -1,3 +1,4 @@ +import {FeatureFlags} from "@actions/expressions/features"; import {TemplateContext} from "../../templates/template-context.js"; import {MappingToken} from "../../templates/tokens/mapping-token.js"; import {SequenceToken} from "../../templates/tokens/sequence-token.js"; @@ -55,7 +56,8 @@ export function convertOn(context: TemplateContext, token: TemplateToken): Event // Schedule is the only event that can be a sequence, handle that separately if (eventName === "schedule") { const scheduleToken = item.value.assertSequence(`event ${eventName}`); - result.schedule = convertSchedule(context, scheduleToken); + const featureFlags = context.state["featureFlags"] as FeatureFlags | undefined; + result.schedule = convertSchedule(context, scheduleToken, featureFlags); continue; } @@ -147,25 +149,47 @@ function convertFilter