fix(webhook): resolve matcher order entirely in Terraform

lambdas/functions/webhook/src/runners/dispatch.test.ts

@@ -107,28 +107,30 @@ describe('Dispatcher', () => {
       });
     });
 
-    it('should sort matcher with exact first.', async () => {
+    it('should respect the strict-first order it is given.', async () => {
+      // Terraform emits the matchers already ordered (strict first, then priority),
+      // so the fixture is in that order rather than relying on the dispatcher to sort.
       config = await createConfig(undefined, [
         {
           ...runnerConfig[0],
           matcherConfig: {
-            labelMatchers: [['self-hosted', 'match', 'not-select']],
-            exactMatch: false,
+            labelMatchers: [['self-hosted', 'no-match']],
+            exactMatch: true,
           },
         },
         {
           ...runnerConfig[0],
+          id: 'match',
           matcherConfig: {
-            labelMatchers: [['self-hosted', 'no-match']],
+            labelMatchers: [['self-hosted', 'match']],
             exactMatch: true,
           },
         },
         {
           ...runnerConfig[0],
-          id: 'match',
           matcherConfig: {
-            labelMatchers: [['self-hosted', 'match']],
-            exactMatch: true,
+            labelMatchers: [['self-hosted', 'match', 'not-select']],
+            exactMatch: false,
           },
         },
         runnerConfig[1],
@@ -207,13 +209,14 @@ describe('Dispatcher', () => {
 
     it('random still respects exactMatch priority (never a lower-priority match)', async () => {
       process.env.QUEUE_SELECTION_STRATEGY = 'random';
+      // Ordered as Terraform emits it: strict matchers ahead of loose ones.
       config = await createConfig(undefined, [
-        { ...runnerConfig[0], id: 'loose', matcherConfig: { labelMatchers: [['self-hosted']], exactMatch: false } },
         {
           ...runnerConfig[0],
           id: 'exact',
           matcherConfig: { labelMatchers: [['self-hosted', 'any']], exactMatch: true },
         },
+        { ...runnerConfig[0], id: 'loose', matcherConfig: { labelMatchers: [['self-hosted']], exactMatch: false } },
       ]);
       const rand = vi.spyOn(Math, 'random').mockReturnValue(0.99);
       await dispatch(jobEvent(['self-hosted', 'any']), 'workflow_job', config);
@@ -223,10 +226,11 @@ describe('Dispatcher', () => {
 
     it('all dispatches to every equally-matching queue but not lower-priority ones', async () => {
       process.env.QUEUE_SELECTION_STRATEGY = 'all';
+      // Ordered as Terraform emits it: strict matchers ahead of loose ones.
       config = await createConfig(undefined, [
-        { ...runnerConfig[0], id: 'loose', matcherConfig: { labelMatchers: [['self-hosted']], exactMatch: false } },
         { ...runnerConfig[0], id: 'q1', matcherConfig: { labelMatchers: [['self-hosted', 'any']], exactMatch: true } },
         { ...runnerConfig[0], id: 'q2', matcherConfig: { labelMatchers: [['self-hosted', 'any']], exactMatch: true } },
+        { ...runnerConfig[0], id: 'loose', matcherConfig: { labelMatchers: [['self-hosted']], exactMatch: false } },
       ]);
       await dispatch(jobEvent(['self-hosted', 'any']), 'workflow_job', config);
       expect(sendActionRequest).toHaveBeenCalledTimes(2);

lambdas/functions/webhook/src/runners/dispatch.ts

@@ -48,13 +48,11 @@ async function handleWorkflowJob(
       `Run ID: ${body.workflow_job.run_id}, Labels: ${JSON.stringify(body.workflow_job.labels)}`,
   );
 
-  // Sort queues by priority (exact/bidirectional match first), as before.
-  matcherConfig.sort((a, b) => {
-    const aStrict = a.matcherConfig.bidirectionalLabelMatch || a.matcherConfig.exactMatch;
-    const bStrict = b.matcherConfig.bidirectionalLabelMatch || b.matcherConfig.exactMatch;
-    return aStrict === bStrict ? 0 : aStrict ? -1 : 1;
-  });
-
+  // matcherConfig arrives already in dispatch order: strict matchers (exact or
+  // bidirectional) first, then ascending priority. That order is built in
+  // modules/webhook/webhook.tf and serialised to SSM, so there is nothing to sort
+  // here. Sorting locally would also mutate the config held on the ConfigLoader
+  // singleton, which outlives a single invocation.
   const allLabels = body.workflow_job.labels;
   const ghrLabels = allLabels.filter((l) => l.startsWith('ghr-'));
   const sanitizedGhrLabels = sanitizeGhrLabels(ghrLabels);

lambdas/functions/webhook/src/sqs/index.ts

@@ -22,6 +22,13 @@ export interface ActionRequestMessage {
 export interface MatcherConfig {
   labelMatchers: string[][];
   exactMatch: boolean;
+  /**
+   * Evaluation order, lower is preferred. Applied in Terraform, which emits the
+   * matcher list already ordered by it (see modules/webhook/webhook.tf); the
+   * dispatcher does not sort on it. Declared here for fidelity with the config
+   * the lambda actually receives.
+   */
+  priority?: number;
   bidirectionalLabelMatch?: boolean;
   enableDynamicLabels?: boolean;
   ec2DynamicLabelsPolicy?: Ec2DynamicLabelsPolicy | null;

modules/multi-runner/variables.tf

@@ -285,7 +285,7 @@ variable "multi_runner_config" {
         labelMatchers: "The list of list of labels supported by the runner configuration. `[[self-hosted, linux, x64, example]]`"
         exactMatch: "DEPRECATED: Use `bidirectionalLabelMatch` instead. If set to true all labels in the workflow job must match the GitHub labels (os, architecture and `self-hosted`). When false if __any__ workflow label matches it will trigger the webhook. Note: this only checks that workflow labels are a subset of runner labels, not the reverse."
         bidirectionalLabelMatch: "If set to true, the runner labels and workflow job labels must be an exact two-way match (same set, any order, no extras or missing labels). This is stricter than `exactMatch` which only checks that workflow labels are a subset of runner labels. When false, if __any__ workflow label matches it will trigger the webhook."
-        priority: "If set it defines the priority of the matcher, the matcher with the lowest priority will be evaluated first. Default is 999, allowed values 0-999."
+        priority: "If set it defines the priority of the matcher, the matcher with the lowest priority will be evaluated first. Matchers with an exact or bidirectional label match are always evaluated ahead of the rest, regardless of priority. Default is 999."
         enableDynamicLabels: "Experimental! When true the dispatcher allows `ghr-*` dynamic labels for jobs routed to this runner. Default false."
         ec2DynamicLabelsPolicy: "Optional policy for `ghr-ec2-*` labels evaluated by the dispatcher. Only effective when `enableDynamicLabels = true`. Jobs whose EC2 dynamic labels violate every matching runner's policy are rejected with a 202 (a warning is logged). Evaluation: keys in `blocked_keys` are always rejected; keys in `restricted_keys` are allowed only when their value passes the rule; unlisted keys are allowed. Schema: `{ blocked_keys = [<key>], restricted_keys = { <key> = { allowed = [globs], denied = [globs], max = number|string } } }`. Keys use the dynamic label suffix, e.g. `instance-type` for `ghr-ec2-instance-type`."
       }

modules/webhook/tests/matcher_order.tftest.hcl

@@ -0,0 +1,118 @@
+# The dispatcher consumes runner_matcher_config_sorted as-is and does no sorting
+# of its own, so the ordering guarantee lives here.
+
+mock_provider "aws" {
+  mock_data "aws_iam_policy_document" {
+    defaults = {
+      json = "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Effect\":\"Allow\",\"Principal\":{\"Service\":\"lambda.amazonaws.com\"},\"Action\":\"sts:AssumeRole\"}]}"
+    }
+  }
+}
+
+variables {
+  prefix     = "test"
+  aws_region = "eu-west-1"
+
+  # Reference the zip via S3 so the plan does not need a local file on disk.
+  lambda_s3_bucket      = "test-bucket"
+  webhook_lambda_s3_key = "webhook.zip"
+
+  sqs_workflow_job_queue = {
+    arn  = "arn:aws:sqs:eu-west-1:123456789012:workflow-job"
+    id   = "https://sqs.eu-west-1.amazonaws.com/123456789012/workflow-job"
+    url  = "https://sqs.eu-west-1.amazonaws.com/123456789012/workflow-job"
+    name = "workflow-job"
+  }
+
+  ssm_paths = {
+    root    = "/test"
+    webhook = "webhook"
+  }
+
+  eventbridge = {
+    enable = false
+  }
+
+  github_app_parameters = {
+    webhook_secret = {
+      name = "/test/webhook_secret"
+      arn  = "arn:aws:ssm:eu-west-1:123456789012:parameter/test/webhook_secret"
+    }
+  }
+
+  runner_matcher_config = {
+    loose_low = {
+      arn = "arn:aws:sqs:eu-west-1:123456789012:loose-low"
+      id  = "loose-low"
+      matcherConfig = {
+        labelMatchers = [["self-hosted"]]
+        exactMatch    = false
+        priority      = 1
+      }
+    }
+    loose_default = {
+      arn = "arn:aws:sqs:eu-west-1:123456789012:loose-default"
+      id  = "loose-default"
+      matcherConfig = {
+        labelMatchers = [["self-hosted"]]
+        exactMatch    = false
+      }
+    }
+    # Deliberately wider than the historical 0-999 range: it must still sort after
+    # 999 by magnitude, not ahead of it by leading digit.
+    loose_wide = {
+      arn = "arn:aws:sqs:eu-west-1:123456789012:loose-wide"
+      id  = "loose-wide"
+      matcherConfig = {
+        labelMatchers = [["self-hosted"]]
+        exactMatch    = false
+        priority      = 1000
+      }
+    }
+    exact_high = {
+      arn = "arn:aws:sqs:eu-west-1:123456789012:exact-high"
+      id  = "exact-high"
+      matcherConfig = {
+        labelMatchers = [["self-hosted", "exact"]]
+        exactMatch    = true
+        priority      = 500
+      }
+    }
+    bidirectional_low = {
+      arn = "arn:aws:sqs:eu-west-1:123456789012:bidirectional-low"
+      id  = "bidirectional-low"
+      matcherConfig = {
+        labelMatchers           = [["self-hosted", "bidi"]]
+        exactMatch              = false
+        bidirectionalLabelMatch = true
+        priority                = 5
+      }
+    }
+  }
+}
+
+run "strict_matchers_are_ordered_ahead_of_loose_ones" {
+  command = plan
+
+  assert {
+    condition = [for c in local.runner_matcher_config_sorted : c.id] == [
+      "bidirectional-low",
+      "exact-high",
+      "loose-low",
+      "loose-default",
+      "loose-wide",
+    ]
+    error_message = "Matchers must be ordered strict first, then by ascending priority. Got: ${jsonencode([for c in local.runner_matcher_config_sorted : c.id])}"
+  }
+}
+
+run "priority_beyond_three_digits_sorts_by_magnitude" {
+  command = plan
+
+  # Guards the zero-padding width: too narrow a pad sorts 1000 ahead of 999
+  # because '1' < '9' lexicographically.
+  assert {
+    condition     = index([for c in local.runner_matcher_config_sorted : c.id], "loose-wide") > index([for c in local.runner_matcher_config_sorted : c.id], "loose-default")
+    error_message = "priority 1000 must sort after the 999 default, not ahead of it."
+  }
+}

modules/webhook/variables.tf

@@ -23,7 +23,7 @@ variable "tags" {
 }
 
 variable "runner_matcher_config" {
-  description = "SQS queue to publish accepted build events based on the runner type. When exact match is disabled the webhook accepts the event if one of the workflow job labels is part of the matcher. The priority defines the order the matchers are applied. Optional `matcherConfig.enableDynamicLabels` and `matcherConfig.ec2DynamicLabelsPolicy` are evaluated by the dispatcher to gate `ghr-ec2-*` labels per runner. The policy supports `blocked_keys = [<key>]` and `restricted_keys = { <key> = { allowed = [globs], denied = [globs], max = number|string } }`; keys use the `ghr-ec2-*` suffix form, for example `instance-type`."
+  description = "SQS queue to publish accepted build events based on the runner type. When exact match is disabled the webhook accepts the event if one of the workflow job labels is part of the matcher. The priority defines the order the matchers are applied; matchers with an exact or bidirectional label match are evaluated first, then by ascending priority. The order is resolved here and consumed as-is by the dispatcher. Optional `matcherConfig.enableDynamicLabels` and `matcherConfig.ec2DynamicLabelsPolicy` are evaluated by the dispatcher to gate `ghr-ec2-*` labels per runner. The policy supports `blocked_keys = [<key>]` and `restricted_keys = { <key> = { allowed = [globs], denied = [globs], max = number|string } }`; keys use the `ghr-ec2-*` suffix form, for example `instance-type`."
   type = map(object({
     arn = string
     id  = string
@@ -36,10 +36,6 @@ variable "runner_matcher_config" {
       ec2DynamicLabelsPolicy  = optional(any, null)
     })
   }))
-  validation {
-    condition     = try(var.runner_matcher_config.matcherConfig.priority, 999) >= 0 && try(var.runner_matcher_config.matcherConfig.priority, 999) < 1000
-    error_message = "The priority of the matcher must be between 0 and 999."
-  }
 }
 
 variable "lambda_zip" {

modules/webhook/webhook.tf

@@ -1,6 +1,17 @@
 locals {
-  # config with combined key and order
-  runner_matcher_config = { for k, v in var.runner_matcher_config : format("%03d-%s", v.matcherConfig.priority, k) => merge(v, { key = k }) }
+  # Config keyed so that lexicographic sorting yields the dispatch order: strict
+  # matchers (exact or bidirectional) first, then ascending priority, then the
+  # config key as a stable tiebreaker. The dispatcher consumes this order as-is
+  # and does no sorting of its own.
+  #
+  # Priority is zero padded so string sorting matches numeric sorting. The width
+  # is generous on purpose: a value wider than the padding sorts by its leading
+  # digit instead of its magnitude, which would put 1000 ahead of 999.
+  runner_matcher_config = { for k, v in var.runner_matcher_config :
+    format("%d-%010d-%s",
+      (v.matcherConfig.bidirectionalLabelMatch || v.matcherConfig.exactMatch) ? 0 : 1,
+    v.matcherConfig.priority, k) => merge(v, { key = k })
+  }
 
   # sorted list
   runner_matcher_config_sorted = [for k in sort(keys(local.runner_matcher_config)) : local.runner_matcher_config[k]]