refactor(plugins): centralize on= validation in resolveTrigger

resolveTrigger now takes { name, validModes }, performs the unknown-mode
and missing-path/pattern checks itself, prints the error via output.error,
and returns null. Each plugin shrinks to one call + early-return; heal
passes its narrower mode list as { validModes: ['fail', 'file', 'url'] }.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: DavertMik

lib/plugin/aiTrace.js

@@ -20,8 +20,6 @@ import {
 
 const supportedHelpers = Container.STANDARD_ACTING_HELPERS
 
-const VALID_MODES = new Set(['fail', 'test', 'step', 'file', 'url'])
-
 const defaultConfig = {
   on: 'step',
   deleteSuccessful: false,
@@ -79,20 +77,8 @@ export default function (config = {}) {
   let helper
 
   const cliArgs = parsePluginArgs(config._args)
-  const trigger = resolveTrigger(cliArgs, config, { on: defaultConfig.on })
-
-  if (!VALID_MODES.has(trigger.on)) {
-    output.error(`aiTrace: unknown on="${trigger.on}". Valid: fail, test, step, file, url`)
-    return
-  }
-  if (trigger.on === 'file' && !trigger.path) {
-    output.error('aiTrace:on=file requires path=. Example: -p aiTrace:on=file:path=tests/foo.js')
-    return
-  }
-  if (trigger.on === 'url' && !trigger.pattern) {
-    output.error('aiTrace:on=url requires pattern=. Example: -p aiTrace:on=url:pattern=/users/*')
-    return
-  }
+  const trigger = resolveTrigger(cliArgs, config, { on: defaultConfig.on }, { name: 'aiTrace' })
+  if (!trigger) return
 
   config = Object.assign(defaultConfig, config)
 

lib/plugin/heal.js

@@ -19,8 +19,6 @@ import {
 } from '../utils/pluginParser.js'
 
 
-const VALID_MODES = new Set(['fail', 'file', 'url'])
-
 const defaultConfig = {
   on: 'fail',
   healLimit: 2,
@@ -64,20 +62,11 @@ export default function (config = {}) {
   }
 
   const cliArgs = parsePluginArgs(config._args)
-  const trigger = resolveTrigger(cliArgs, config, { on: defaultConfig.on })
-
-  if (!VALID_MODES.has(trigger.on)) {
-    output.error(`heal: unsupported on="${trigger.on}". Valid: fail, file, url`)
-    return
-  }
-  if (trigger.on === 'file' && !trigger.path) {
-    output.error('heal:on=file requires path=. Example: -p heal:on=file:path=tests/foo.js')
-    return
-  }
-  if (trigger.on === 'url' && !trigger.pattern) {
-    output.error('heal:on=url requires pattern=. Example: -p heal:on=url:pattern=/users/*')
-    return
-  }
+  const trigger = resolveTrigger(cliArgs, config, { on: defaultConfig.on }, {
+    name: 'heal',
+    validModes: ['fail', 'file', 'url'],
+  })
+  if (!trigger) return
 
   let currentTest = null
   let currentStep = null

lib/plugin/pause.js

@@ -10,8 +10,6 @@ import {
   getBrowserHelper,
 } from '../utils/pluginParser.js'
 
-const VALID_MODES = new Set(['fail', 'test', 'step', 'file', 'url'])
-
 /**
  * Pauses test execution interactively. Replaces the legacy `pauseOnFail`
  * plugin. The default `on=fail` matches the old `pauseOnFail` behavior.
@@ -46,12 +44,8 @@ const VALID_MODES = new Set(['fail', 'test', 'step', 'file', 'url'])
  */
 export default function (config = {}) {
   const cliArgs = parsePluginArgs(config._args)
-  const trigger = resolveTrigger(cliArgs, config, { on: 'fail' })
-
-  if (!VALID_MODES.has(trigger.on)) {
-    output.error(`pause: unknown on="${trigger.on}". Valid: fail, test, step, file, url`)
-    return
-  }
+  const trigger = resolveTrigger(cliArgs, config, { on: 'fail' }, { name: 'pause' })
+  if (!trigger) return
 
   switch (trigger.on) {
     case 'fail':
@@ -61,16 +55,8 @@ export default function (config = {}) {
     case 'step':
       return initStepMode()
     case 'file':
-      if (!trigger.path) {
-        output.error('pause:on=file requires path=. Example: -p pause:on=file:path=tests/foo.js')
-        return
-      }
       return initFileMode(trigger.path, trigger.line)
     case 'url':
-      if (!trigger.pattern) {
-        output.error('pause:on=url requires pattern=. Example: -p pause:on=url:pattern=/users/*')
-        return
-      }
       return initUrlMode(trigger.pattern)
   }
 }

lib/plugin/screenshot.js

@@ -31,8 +31,6 @@ const defaultConfig = {
   ignoreSteps: [],
 }
 
-const VALID_MODES = new Set(['fail', 'test', 'step', 'file', 'url'])
-
 /**
  * Saves screenshots from the browser at points triggered by `on=`.
  *
@@ -85,12 +83,8 @@ export default function (config = {}) {
   if (!helper) return
 
   const cliArgs = parsePluginArgs(config._args)
-  const trigger = resolveTrigger(cliArgs, config, { on: defaultConfig.on })
-
-  if (!VALID_MODES.has(trigger.on)) {
-    output.error(`screenshot: unknown on="${trigger.on}". Valid: fail, test, step, file, url`)
-    return
-  }
+  const trigger = resolveTrigger(cliArgs, config, { on: defaultConfig.on }, { name: 'screenshot' })
+  if (!trigger) return
 
   const helpers = Container.helpers()
   const options = Object.assign({}, defaultConfig, helper.options, config)
@@ -119,16 +113,8 @@ export default function (config = {}) {
     case 'step':
       return wireOnStep(options, () => true)
     case 'file':
-      if (!trigger.path) {
-        output.error('screenshot:on=file requires path=. Example: -p screenshot:on=file:path=tests/foo.js')
-        return
-      }
       return wireOnStep(options, step => matchStepFile(step, trigger.path, trigger.line))
     case 'url':
-      if (!trigger.pattern) {
-        output.error('screenshot:on=url requires pattern=. Example: -p screenshot:on=url:pattern=/users/*')
-        return
-      }
       return wireOnUrl(options, trigger.pattern)
   }
 }

lib/utils/pluginParser.js

@@ -1,8 +1,10 @@
 import Container from '../container.js'
+import output from '../output.js'
 
 const supportedHelpers = Container.STANDARD_ACTING_HELPERS
 
 const RESERVED_KEYS = new Set(['on', 'path', 'line', 'pattern'])
+const ALL_MODES = ['fail', 'test', 'step', 'file', 'url']
 
 /**
  * Parse a plugin's _args (from CLI `-p plugin:key=value:key=value`) into a flat dict.
@@ -44,12 +46,37 @@ function coerce(v) {
 }
 
 /**
- * Compose CLI args > config > defaults into a normalized trigger spec.
- * Returns `{ on, path, line, pattern, ...rest }`. `line` is coerced to a number.
+ * Compose CLI args > config > defaults into a normalized trigger spec, then
+ * validate it. Returns `{ on, path, line, pattern, ...rest }` with `line`
+ * coerced to a number, or `null` if validation failed (an error is printed).
+ *
+ * @param {object} cliArgs           — output of parsePluginArgs(config._args)
+ * @param {object} config            — full plugin config object
+ * @param {object} defaults          — fallback values, e.g. `{ on: 'fail' }`
+ * @param {object} options
+ * @param {string} options.name      — plugin name, used in error messages
+ * @param {string[]} [options.validModes] — accepted values for `on`
+ *                                          (default: fail, test, step, file, url)
  */
-export function resolveTrigger(cliArgs = {}, config = {}, defaults = {}) {
+export function resolveTrigger(cliArgs = {}, config = {}, defaults = {}, options = {}) {
+  const { name = 'plugin', validModes = ALL_MODES } = options
   const merged = { ...defaults, ...pickKnown(config), ...cliArgs }
   if (merged.line != null) merged.line = parseInt(merged.line, 10)
+
+  const valid = new Set(validModes)
+  if (!valid.has(merged.on)) {
+    output.error(`${name}: unknown on="${merged.on}". Valid: ${validModes.join(', ')}`)
+    return null
+  }
+  if (merged.on === 'file' && !merged.path) {
+    output.error(`${name}:on=file requires path=. Example: -p ${name}:on=file:path=tests/foo.js`)
+    return null
+  }
+  if (merged.on === 'url' && !merged.pattern) {
+    output.error(`${name}:on=url requires pattern=. Example: -p ${name}:on=url:pattern=/users/*`)
+    return null
+  }
+
   return merged
 }