From 8507419ebffe067a67d03fb7fdc74773c1595a98 Mon Sep 17 00:00:00 2001 From: eric sciple Date: Mon, 8 Dec 2025 13:44:56 -0600 Subject: [PATCH] Add missing activity types for pull_request and pull_request_target (#242) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #51 Added the following activity types to pull_request and pull_request_target: - milestoned - demilestoned - enqueued - dequeued These types were missing from workflow-v1.0.json but are valid workflow triggers per GitHub docs. Also added schema-sync.test.ts to ensure activity types in workflow-v1.0.json stay in sync with webhooks.json. The test: - Checks both directions (webhooks→schema and schema→webhooks) - Has WEBHOOK_ONLY for types not valid as workflow triggers: - check_suite: requested, rerequested - registry_package: default - Has SCHEMA_ONLY for types valid in workflows but not in webhooks: - registry_package: updated - Has NAME_MAPPINGS for naming differences: - project_column: edited (webhook) ↔ updated (schema) - Provides actionable error messages when mismatches are found --- docs/json-data-files.md | 47 +++++ workflow-parser/src/schema-sync.test.ts | 183 ++++++++++++++++++ workflow-parser/src/workflow-v1.0.json | 16 +- .../testdata/reader/events-mapping-all.yml | 16 ++ 4 files changed, 258 insertions(+), 4 deletions(-) create mode 100644 workflow-parser/src/schema-sync.test.ts diff --git a/docs/json-data-files.md b/docs/json-data-files.md index 94ff17e..6230cc9 100644 --- a/docs/json-data-files.md +++ b/docs/json-data-files.md @@ -148,3 +148,50 @@ Only `name`, `description`, and `childParamsGroups` are kept — these are used To compare all fields vs stripped, run `npm run update-webhooks -- --all` and diff the `.all.json` files against the regular ones. See `EVENT_ACTION_FIELDS` and `BODY_PARAM_FIELDS` in `script/webhooks/index.ts` to modify what gets stripped. + +## Schema Synchronization + +The `workflow-v1.0.json` schema defines which activity types are valid for each workflow trigger event. A test in `workflow-parser/src/schema-sync.test.ts` verifies these stay in sync with `webhooks.json`. + +### When the Test Fails + +If the schema-sync test fails, you'll see an error like: + +``` +Event "pull_request" is missing activity type "new_activity" in workflow-v1.0.json +``` + +**To resolve:** + +1. Check [Events that trigger workflows](https://docs.github.com/en/actions/writing-workflows/choosing-when-your-workflow-runs/events-that-trigger-workflows) to verify the activity type is a valid workflow trigger: + - Find the event section (e.g., "pull_request") + - Look at the "Activity types" table — it lists which types can be used in `on..types` + - If the type is listed there, it's a valid workflow trigger + - If the type only appears in webhook docs but NOT in the workflow trigger docs, it's webhook-only + +2. If it IS a valid workflow trigger: + - Edit `workflow-parser/src/workflow-v1.0.json` + - Find the `-activity-type` definition (e.g., `pull-request-activity-type`) + - Add the new activity type to `allowed-values` + - Update the `description` in `-activity` to list all types + - Run `npm test` to regenerate the minified JSON + +3. If it is NOT a valid workflow trigger (webhook-only): + - Edit `workflow-parser/src/schema-sync.test.ts` + - Add the type to `WEBHOOK_ONLY` for that event + +### Known Discrepancies + +The test tracks several types of known discrepancies: + +| Category | Purpose | Example | +|----------|---------|---------| +| `WEBHOOK_ONLY` | Types in webhooks that aren't valid workflow triggers | `check_suite.requested` | +| `SCHEMA_ONLY` | Types valid for workflows but missing from webhooks | `registry_package.updated` | +| `NAME_MAPPINGS` | Different names for the same concept | `project_column`: webhook uses `edited`, schema uses `updated` | + +### Bidirectional Checking + +The test checks both directions: +- **webhooks → schema**: Ensures all webhook activity types are in the schema (or listed in `WEBHOOK_ONLY`) +- **schema → webhooks**: Ensures the schema doesn't have types that don't exist in webhooks (or listed in `SCHEMA_ONLY` or `NAME_MAPPINGS`) diff --git a/workflow-parser/src/schema-sync.test.ts b/workflow-parser/src/schema-sync.test.ts new file mode 100644 index 0000000..c57157f --- /dev/null +++ b/workflow-parser/src/schema-sync.test.ts @@ -0,0 +1,183 @@ +import * as fs from "fs"; + +/** + * This test ensures that activity types in workflow-v1.0.json stay in sync with + * the webhooks.json file from the languageservice package. + * + * When this test fails, it means new activity types were added to webhooks.json + * that need to be handled. See docs/json-data-files.md for detailed instructions. + * + * Quick reference for fixing failures: + * 1. Check https://docs.github.com/en/actions/writing-workflows/choosing-when-your-workflow-runs/events-that-trigger-workflows + * Find the event and look at its "Activity types" table to see if the type is a valid workflow trigger. + * 2. If the activity type IS a valid workflow trigger: + * → Add it to the corresponding *-activity-type definition in workflow-v1.0.json + * 3. If the activity type is webhook-only (not in workflow docs): + * → Add it to the WEBHOOK_ONLY list below + * 4. If there's a naming difference between webhook and schema: + * → Add it to the NAME_MAPPINGS list below + * 5. If the schema has a type not in webhooks.json: + * → Add it to the SCHEMA_ONLY list below + */ + +describe("schema-sync", () => { + // Activity types that exist in webhooks.json but are intentionally NOT + // supported as workflow triggers. These will be ignored when checking + // webhooks → schema direction. + const WEBHOOK_ONLY: Record = { + // check_suite: requested and rerequested are webhook-only, not valid workflow triggers + // See: https://docs.github.com/en/actions/writing-workflows/choosing-when-your-workflow-runs/events-that-trigger-workflows#check_suite + check_suite: ["requested", "rerequested"], + + // registry_package: "default" is a webhook concept, not a workflow trigger type + registry_package: ["default"] + }; + + // Activity types that exist in workflow schema but are intentionally NOT + // in webhooks.json (schema-only types). These will be ignored when checking + // schema → webhooks direction. + const SCHEMA_ONLY: Record = { + // registry_package: "updated" is a valid workflow trigger per GitHub docs + // but doesn't exist in webhooks.json (webhooks only has "published" and "default") + // See: https://docs.github.com/en/actions/writing-workflows/choosing-when-your-workflow-runs/events-that-trigger-workflows#registry_package + registry_package: ["updated"] + }; + + // Known naming differences between webhooks.json and workflow-v1.0.json. + // Key: event name, Value: { webhook: "webhookName", schema: "schemaName" } + // These are treated as equivalent when comparing in both directions. + const NAME_MAPPINGS: Record> = { + // project_column: webhooks.json uses "edited" but workflow triggers use "updated" + // This is a known naming difference - they represent the same action + project_column: [{webhook: "edited", schema: "updated"}] + }; + + it("activity types in workflow-v1.0.json match webhooks.json", () => { + // Load webhooks.json (relative path from the test runner CWD which is the package root) + const webhooksPath = "../languageservice/src/context-providers/events/webhooks.json"; + const webhooks = JSON.parse(fs.readFileSync(webhooksPath, "utf-8")) as Record>; + + // Load workflow-v1.0.json + const schemaPath = "./src/workflow-v1.0.json"; + const schema = JSON.parse(fs.readFileSync(schemaPath, "utf-8")) as { + definitions: Record; + }; + + const mismatches: string[] = []; + + // Build mapping helpers for each event + const getWebhookToSchemaMapping = (eventName: string): Map => { + const map = new Map(); + for (const mapping of NAME_MAPPINGS[eventName] || []) { + map.set(mapping.webhook, mapping.schema); + } + return map; + }; + + const getSchemaToWebhookMapping = (eventName: string): Map => { + const map = new Map(); + for (const mapping of NAME_MAPPINGS[eventName] || []) { + map.set(mapping.schema, mapping.webhook); + } + return map; + }; + + // Check both directions for each event + for (const [eventName, eventData] of Object.entries(webhooks)) { + const webhookTypes = Object.keys(eventData); + if (webhookTypes.length === 0) continue; + + const schemaTypeName = `${eventName.replace(/_/g, "-")}-activity-type`; + const schemaDef = schema.definitions[schemaTypeName]; + + // If there's no activity type definition in the schema, this event + // doesn't support activity types in workflows (e.g., push, pull) + if (!schemaDef || !schemaDef["allowed-values"]) continue; + + const schemaTypes = new Set(schemaDef["allowed-values"]); + const webhookOnly = new Set(WEBHOOK_ONLY[eventName] || []); + const schemaOnly = new Set(SCHEMA_ONLY[eventName] || []); + const webhookToSchema = getWebhookToSchemaMapping(eventName); + const schemaToWebhook = getSchemaToWebhookMapping(eventName); + + // Direction 1: webhooks → schema + // Check that each webhook type exists in schema (or has a mapping, or is webhook-only) + for (const webhookType of webhookTypes) { + if (webhookOnly.has(webhookType)) continue; + + const mappedSchemaType = webhookToSchema.get(webhookType); + if (mappedSchemaType) { + // Has a mapping - check the mapped name exists in schema + if (!schemaTypes.has(mappedSchemaType)) { + mismatches.push( + `Event "${eventName}": webhook type "${webhookType}" maps to "${mappedSchemaType}" but "${mappedSchemaType}" not found in schema` + ); + } + } else { + // No mapping - check the type exists directly + if (!schemaTypes.has(webhookType)) { + mismatches.push( + `Event "${eventName}": missing activity type "${webhookType}" in workflow-v1.0.json (exists in webhooks.json)` + ); + } + } + } + + // Direction 2: schema → webhooks + // Check that each schema type exists in webhooks (or has a mapping, or is schema-only) + const webhookTypesSet = new Set(webhookTypes); + for (const schemaType of schemaTypes) { + if (schemaOnly.has(schemaType)) continue; + + const mappedWebhookType = schemaToWebhook.get(schemaType); + if (mappedWebhookType) { + // Has a mapping - check the mapped name exists in webhooks + if (!webhookTypesSet.has(mappedWebhookType)) { + mismatches.push( + `Event "${eventName}": schema type "${schemaType}" maps to "${mappedWebhookType}" but "${mappedWebhookType}" not found in webhooks.json` + ); + } + } else { + // No mapping - check the type exists directly + if (!webhookTypesSet.has(schemaType)) { + mismatches.push( + `Event "${eventName}": extra activity type "${schemaType}" in workflow-v1.0.json (not in webhooks.json)` + ); + } + } + } + + // Check that the description mentions all allowed values + const activityDefName = `${eventName.replace(/_/g, "-")}-activity`; + const activityDef = schema.definitions[activityDefName]; + if (activityDef?.description) { + for (const schemaType of schemaTypes) { + if (!activityDef.description.includes(`\`${schemaType}\``)) { + mismatches.push( + `Event "${eventName}": description in "${activityDefName}" is missing activity type \`${schemaType}\`` + ); + } + } + } + } + + if (mismatches.length > 0) { + const errorMessage = [ + "Activity type mismatches found between webhooks.json and workflow-v1.0.json:", + "", + ...mismatches, + "", + "To fix these mismatches:", + "1. Check GitHub docs: https://docs.github.com/en/actions/writing-workflows/choosing-when-your-workflow-runs/events-that-trigger-workflows", + "2. Verify the activity type is valid for workflow triggers", + "3. Update the *-activity-type definition in workflow-parser/src/workflow-v1.0.json", + "4. Update the description to list all supported activity types", + "5. If there's a naming difference, add it to NAME_MAPPINGS in schema-sync.test.ts", + "6. If the type is webhook-only, add it to WEBHOOK_ONLY", + "7. If the type is schema-only, add it to SCHEMA_ONLY" + ].join("\n"); + + throw new Error(errorMessage); + } + }); +}); diff --git a/workflow-parser/src/workflow-v1.0.json b/workflow-parser/src/workflow-v1.0.json index 4d09751..12a38e7 100644 --- a/workflow-parser/src/workflow-v1.0.json +++ b/workflow-parser/src/workflow-v1.0.json @@ -856,7 +856,7 @@ } }, "pull-request-activity": { - "description": "The types of pull request activity that trigger the workflow. Supported activity types: `assigned`, `unassigned`, `labeled`, `unlabeled`, `opened`, `edited`, `closed`, `reopened`, `synchronize`, `converted_to_draft`, `ready_for_review`, `locked`, `unlocked`, `review_requested`, `review_request_removed`, `auto_merge_enabled`, `auto_merge_disabled`.", + "description": "The types of pull request activity that trigger the workflow. Supported activity types: `assigned`, `unassigned`, `labeled`, `unlabeled`, `opened`, `edited`, `closed`, `reopened`, `synchronize`, `converted_to_draft`, `locked`, `unlocked`, `enqueued`, `dequeued`, `milestoned`, `demilestoned`, `ready_for_review`, `review_requested`, `review_request_removed`, `auto_merge_enabled`, `auto_merge_disabled`.", "one-of": [ "pull-request-activity-type", "pull-request-activity-types" @@ -879,9 +879,13 @@ "reopened", "synchronize", "converted_to_draft", - "ready_for_review", "locked", "unlocked", + "enqueued", + "dequeued", + "milestoned", + "demilestoned", + "ready_for_review", "review_requested", "review_request_removed", "auto_merge_enabled", @@ -1004,7 +1008,7 @@ } }, "pull-request-target-activity": { - "description": "The types of pull request activity that trigger the workflow. Supported activity types: `assigned`, `unassigned`, `labeled`, `unlabeled`, `opened`, `edited`, `closed`, `reopened`, `synchronize`, `converted_to_draft`, `ready_for_review`, `locked`, `unlocked`, `review_requested`, `review_request_removed`, `auto_merge_enabled`, `auto_merge_disabled`.", + "description": "The types of pull request activity that trigger the workflow. Supported activity types: `assigned`, `unassigned`, `labeled`, `unlabeled`, `opened`, `edited`, `closed`, `reopened`, `synchronize`, `converted_to_draft`, `locked`, `unlocked`, `enqueued`, `dequeued`, `milestoned`, `demilestoned`, `ready_for_review`, `review_requested`, `review_request_removed`, `auto_merge_enabled`, `auto_merge_disabled`.", "one-of": [ "pull-request-target-activity-type", "pull-request-target-activity-types" @@ -1027,9 +1031,13 @@ "reopened", "synchronize", "converted_to_draft", - "ready_for_review", "locked", "unlocked", + "enqueued", + "dequeued", + "milestoned", + "demilestoned", + "ready_for_review", "review_requested", "review_request_removed", "auto_merge_enabled", diff --git a/workflow-parser/testdata/reader/events-mapping-all.yml b/workflow-parser/testdata/reader/events-mapping-all.yml index 94b26ef..b144587 100644 --- a/workflow-parser/testdata/reader/events-mapping-all.yml +++ b/workflow-parser/testdata/reader/events-mapping-all.yml @@ -120,6 +120,8 @@ on: - unassigned - labeled - unlabeled + - milestoned + - demilestoned - opened - edited - closed @@ -129,6 +131,8 @@ on: - ready_for_review - locked - unlocked + - enqueued + - dequeued - review_requested - review_request_removed - auto_merge_enabled @@ -160,6 +164,8 @@ on: - unassigned - labeled - unlabeled + - milestoned + - demilestoned - opened - edited - closed @@ -169,6 +175,8 @@ on: - ready_for_review - locked - unlocked + - enqueued + - dequeued - review_requested - review_request_removed - auto_merge_enabled @@ -386,6 +394,8 @@ jobs: "unassigned", "labeled", "unlabeled", + "milestoned", + "demilestoned", "opened", "edited", "closed", @@ -395,6 +405,8 @@ jobs: "ready_for_review", "locked", "unlocked", + "enqueued", + "dequeued", "review_requested", "review_request_removed", "auto_merge_enabled", @@ -441,6 +453,8 @@ jobs: "unassigned", "labeled", "unlabeled", + "milestoned", + "demilestoned", "opened", "edited", "closed", @@ -450,6 +464,8 @@ jobs: "ready_for_review", "locked", "unlocked", + "enqueued", + "dequeued", "review_requested", "review_request_removed", "auto_merge_enabled",