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 <jiakou@microsoft.com>
This commit is contained in:
Angel Kou
2026-03-05 17:59:56 -08:00
committed by GitHub
parent 8c0a3a947b
commit a7f581bde5
11 changed files with 267 additions and 21 deletions
+2 -1
View File
@@ -54,7 +54,8 @@ describe("FeatureFlags", () => {
expect(flags.getEnabledFeatures()).toEqual([
"missingInputsQuickfix",
"blockScalarChompingWarning",
"allowCaseFunction"
"allowCaseFunction",
"allowCronTimezone"
]);
});
});
+8 -1
View File
@@ -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<keyof ExperimentalFeatures, "all">;
const allFeatureKeys: ExperimentalFeatureKey[] = [
"missingInputsQuickfix",
"blockScalarChompingWarning",
"allowCaseFunction"
"allowCaseFunction",
"allowCronTimezone"
];
export class FeatureFlags {
+42
View File
@@ -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");
});
});
+5
View File
@@ -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);
+6 -2
View File
@@ -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 () => {
+2 -1
View File
@@ -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
+137
View File
@@ -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 * * *"
});
});
});
});
+15 -2
View File
@@ -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<WorkflowTemplateConverterOptions> = {
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
};
}
+36 -12
View File
@@ -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<T extends TypesFilterConfig & WorkflowFilterConfig & Vers
return result;
}
function convertSchedule(context: TemplateContext, token: SequenceToken): ScheduleConfig[] | undefined {
function convertSchedule(
context: TemplateContext,
token: SequenceToken,
featureFlags?: FeatureFlags
): ScheduleConfig[] | undefined {
const flags = featureFlags ?? new FeatureFlags();
const allowTimezone = flags.isEnabled("allowCronTimezone");
const result = [] as ScheduleConfig[];
for (const item of token) {
const mappingToken = item.assertMapping(`event schedule`);
if (mappingToken.count == 1) {
const schedule = mappingToken.get(0);
const scheduleKey = schedule.key.assertString(`schedule key`);
if (scheduleKey.value == "cron") {
const cron = schedule.value.assertString(`schedule cron`);
// Validate the cron string
const config: ScheduleConfig = {cron: ""};
let valid = true;
for (const entry of mappingToken) {
const key = entry.key.assertString(`schedule key`);
if (key.value === "cron") {
const cron = entry.value.assertString(`schedule cron`);
if (!isValidCron(cron.value)) {
context.error(cron, "Invalid cron expression. Expected format: '* * * * *' (minute hour day month weekday)");
}
result.push({cron: cron.value});
config.cron = cron.value;
} else if (key.value === "timezone") {
if (allowTimezone) {
const timezone = entry.value.assertString(`schedule timezone`);
config.timezone = timezone.value;
} else {
context.error(key, `Key 'timezone' is not supported`);
valid = false;
}
} else {
context.error(scheduleKey, `Invalid schedule key`);
context.error(key, `Invalid schedule key`);
valid = false;
}
} else {
context.error(mappingToken, "Invalid format for 'schedule'");
}
if (valid && config.cron) {
result.push(config);
} else if (valid && !config.cron) {
context.error(mappingToken, "Missing required key 'cron' in schedule entry");
}
}
@@ -196,6 +196,7 @@ export type SecretConfig = {
export type ScheduleConfig = {
cron: string;
timezone?: string;
};
export type WorkflowFilterConfig = {
+13 -2
View File
@@ -2620,14 +2620,25 @@
"cron-mapping": {
"mapping": {
"properties": {
"cron": "cron-pattern"
"cron": {
"type": "cron-pattern",
"required": true
},
"timezone": "timezone-string"
}
}
},
"cron-pattern": {
"description": "A cron expression that represents a schedule. A scheduled workflow will run at most once every 5 minutes.",
"string": {
"require-non-empty": true
}
},
"timezone-string": {
"description": "A string that represents the time zone a scheduled workflow will run relative to in IANA format (e.g. 'America/New_York' or 'Europe/London'). If omitted, the workflow will run relative to midnight UTC.",
"string": {
"require-non-empty": true
}
}
}
}
}