fix(hooks): share matcher validation instances

2026-07-28 20:10:21 -07:00
parent 774755ef2d
commit ec72d0b57e
17 changed files with 221 additions and 128 deletions
--- a/packages/hooks/hooks-codex/src/config.ts
+++ b/packages/hooks/hooks-codex/src/config.ts
@@ -5,7 +5,11 @@
 * @module @deepseek-ai/dsh-hooks-codex/config
 */

-import { matcherDiagnostic, type MatcherGroup } from '@deepseek-ai/dsh-hook-protocol'
+import {
+  compileMatchers,
+  type CompiledMatchers,
+  type MatcherGroup,
+} from '@deepseek-ai/dsh-hook-protocol'

 /** The five Codex hook points this bridge supports. */
 export const CODEX_EVENTS = ['PreToolUse', 'PostToolUse', 'SessionStart', 'UserPromptSubmit', 'Stop'] as const
@@ -23,6 +27,8 @@ export interface SkippedHook {
 export interface ParsedCodexConfig {
  config: CodexHookConfig
  skipped: SkippedHook[]
+  /** Config-scoped matcher registry; the caller owns and must dispose it. */
+  matchers: CompiledMatchers
 }

 function asObject(value: unknown): Record<string, unknown> | undefined {
@@ -36,7 +42,8 @@ function asObject(value: unknown): Record<string, unknown> | undefined {
 * than failing boot; unsupported or asynchronous hooks are returned in `skipped`. Matcher fields on
 * UserPromptSubmit and Stop are discarded because those events have no matcher subject. A
 * matcher-bearing runnable group with an invalid regex throws a `SyntaxError`, allowing the bridge
- * to reject the complete config before listener registration.
+ * to reject the complete config before listener registration. Validation and runtime matching
+ * share the returned compiled registry; its caller must dispose it.
 * @param raw - the parsed JSON config: a `{ hooks: … }` wrapper or the bare event map.
 * @returns the runnable per-event groups plus the skipped hooks with their reasons.
 */
@@ -45,42 +52,51 @@ export function parseCodexConfig(raw: unknown): ParsedCodexConfig {
  const skipped: SkippedHook[] = []
  const root = asObject(raw)
  const hooksMap = root ? asObject(root.hooks) ?? root : undefined
-  if (!hooksMap) return { config, skipped }
-
-  for (const event of CODEX_EVENTS) {
-    const rawGroups = hooksMap[event]
-    // Matcher-group parsing remains dialect-local because the supported hook
-    // shapes and skip reasons differ from Claude Code's.
-    /* jscpd:ignore-start */
-    if (!Array.isArray(rawGroups)) continue
-    const groups: MatcherGroup[] = []
-    for (const rawGroup of rawGroups) {
-      const group = asObject(rawGroup)
-      if (!group || !Array.isArray(group.hooks)) continue
-      const commands: MatcherGroup['hooks'] = []
-      for (const rawHook of group.hooks) {
-        const hook = asObject(rawHook)
-        if (!hook) continue
-        const type = typeof hook.type === 'string' ? hook.type : 'command'
-        if (type !== 'command') { skipped.push({ event, reason: `unsupported "${type}" hook` }); continue }
-        /* jscpd:ignore-end */
-        if (hook.async === true) { skipped.push({ event, reason: 'async hook' }); continue }
-        if (typeof hook.command !== 'string') continue
-        // Codex accepts `timeout` or the `timeoutSec` alias.
-        const timeout = typeof hook.timeout === 'number' ? hook.timeout
-          : typeof hook.timeoutSec === 'number' ? hook.timeoutSec : undefined
-        commands.push({ command: hook.command, ...timeout !== undefined ? { timeoutSec: timeout } : {} })
+  if (hooksMap) {
+    for (const event of CODEX_EVENTS) {
+      const rawGroups = hooksMap[event]
+      // Matcher-group parsing remains dialect-local because the supported hook
+      // shapes and skip reasons differ from Claude Code's.
+      /* jscpd:ignore-start */
+      if (!Array.isArray(rawGroups)) continue
+      const groups: MatcherGroup[] = []
+      for (const rawGroup of rawGroups) {
+        const group = asObject(rawGroup)
+        if (!group || !Array.isArray(group.hooks)) continue
+        const commands: MatcherGroup['hooks'] = []
+        for (const rawHook of group.hooks) {
+          const hook = asObject(rawHook)
+          if (!hook) continue
+          const type = typeof hook.type === 'string' ? hook.type : 'command'
+          if (type !== 'command') { skipped.push({ event, reason: `unsupported "${type}" hook` }); continue }
+          /* jscpd:ignore-end */
+          if (hook.async === true) { skipped.push({ event, reason: 'async hook' }); continue }
+          if (typeof hook.command !== 'string') continue
+          // Codex accepts `timeout` or the `timeoutSec` alias.
+          const timeout = typeof hook.timeout === 'number' ? hook.timeout
+            : typeof hook.timeoutSec === 'number' ? hook.timeoutSec : undefined
+          commands.push({ command: hook.command, ...timeout !== undefined ? { timeoutSec: timeout } : {} })
+        }
+        if (commands.length === 0) continue
+        const matcher = event === 'UserPromptSubmit' || event === 'Stop'
+          ? undefined
+          : typeof group.matcher === 'string' ? group.matcher : undefined
+        groups.push({ ...matcher !== undefined ? { matcher } : {}, hooks: commands })
      }
-      if (commands.length === 0) continue
-      const matcher = event === 'UserPromptSubmit' || event === 'Stop'
-        ? undefined
-        : typeof group.matcher === 'string' ? group.matcher : undefined
-      const diagnostic = matcherDiagnostic(matcher, 'codex')
-      if (diagnostic !== undefined) throw new SyntaxError(`${diagnostic} on event ${JSON.stringify(event)}`)
-      groups.push({ ...matcher !== undefined ? { matcher } : {}, hooks: commands })
+      if (groups.length > 0) config[event] = groups
    }
-    if (groups.length > 0) config[event] = groups
  }

-  return { config, skipped }
+  const entries = Object.entries(config).flatMap(([event, groups]) => (
+    groups.map(group => ({ event, matcher: group.matcher }))
+  ))
+  const matchers = compileMatchers(new Set(entries.map(entry => entry.matcher)), 'codex')
+  for (const { event, matcher } of entries) {
+    const diagnostic = matchers.diagnostic(matcher)
+    if (diagnostic === undefined) continue
+    matchers.dispose()
+    throw new SyntaxError(`${diagnostic} on event ${JSON.stringify(event)}`)
+  }
+
+  return { config, skipped, matchers }
 }
--- a/packages/hooks/hooks-codex/src/index.ts
+++ b/packages/hooks/hooks-codex/src/index.ts
@@ -25,7 +25,6 @@ import type { PostToolDecision, PreToolDecision, ToolExecution, ToolExecutionRes
 import {
  appendHookInvoked,
  appendHookResult,
-  compileMatchers,
  createDetachedRuns,
  DEFAULT_HOOK_TIMEOUT_MS,
  DEFAULT_STDERR_SUMMARY_MAX_CHARS,
@@ -35,7 +34,7 @@ import {
  type MatcherGroup,
  type MergedHookOutcome,
 } from '@deepseek-ai/dsh-hook-protocol'
-import { parseCodexConfig, type CodexHookConfig } from './config.ts'
+import { parseCodexConfig, type ParsedCodexConfig } from './config.ts'
 /* jscpd:ignore-end */

 export const name = 'hooks-codex'
@@ -84,28 +83,20 @@ export function apply(ctx: Context, config: Config): void {
  const stderrSummaryMaxChars = config.stderrSummaryMaxChars ?? DEFAULT_STDERR_SUMMARY_MAX_CHARS
  assertPositiveInteger('stderrSummaryMaxChars', stderrSummaryMaxChars)
  const defaultTimeoutMs = config.defaultTimeoutMs ?? DEFAULT_HOOK_TIMEOUT_MS
-  let parsed: CodexHookConfig = {}
+  let result: ParsedCodexConfig
  try {
    const raw: unknown = JSON.parse(readFileSync(config.configPath, 'utf8'))
-    const result = parseCodexConfig(raw)
-    parsed = result.config
-    for (const s of result.skipped) {
-      ctx.logger.warn(`hooks-codex: skipping ${s.reason} on ${s.event} (only sync command hooks run)`)
-    }
+    result = parseCodexConfig(raw)
  } catch (error: unknown) {
    ctx.logger.warn(`hooks-codex: could not load hook config "${config.configPath}": ${String(error)} — no hooks registered`)
    return
  }

+  const parsed = result.config
  const model = config.model ?? ''
-
-  // Compile each distinct config matcher once. In particular, rebuilding an
-  // rregex WASM value on every hook point permanently raises the module's WASM
-  // memory high-water mark even when each value is freed.
-  const matchers = compileMatchers(
-    Object.values(parsed).flatMap(groups => groups.map(group => group.matcher)),
-    'codex',
-  )
+  // Parsing validates through this same registry, so no native regex is rebuilt
+  // between config admission and runtime matching.
+  const matchers = result.matchers

  // SessionStart is the one emit-shaped (detached) point Codex has: track its
  // run chains so disposal aborts a still-running hook process and drains the
@@ -120,6 +111,10 @@ export function apply(ctx: Context, config: Config): void {
    }
  }, 'hooks-codex: drain detached hook runs and dispose matchers')

+  for (const s of result.skipped) {
+    ctx.logger.warn(`hooks-codex: skipping ${s.reason} on ${s.event} (only sync command hooks run)`)
+  }
+
  /**
   * Run and fold one configured Codex hook point.
   *
--- a/packages/hooks/hooks-codex/tests/config.spec.ts
+++ b/packages/hooks/hooks-codex/tests/config.spec.ts
@@ -1,5 +1,14 @@
-import { describe, expect, it } from 'vitest'
-import { parseCodexConfig, CODEX_EVENTS } from '@deepseek-ai/dsh-hooks-codex/src/config.ts'
+import { afterEach, describe, expect, it } from 'vitest'
+import { parseCodexConfig as parseRawCodexConfig, CODEX_EVENTS } from '@deepseek-ai/dsh-hooks-codex/src/config.ts'
+
+const matcherSets: Array<ReturnType<typeof parseRawCodexConfig>['matchers']> = []
+afterEach(() => { for (const matchers of matcherSets.splice(0)) matchers.dispose() })
+
+function parseCodexConfig(...args: Parameters<typeof parseRawCodexConfig>): ReturnType<typeof parseRawCodexConfig> {
+  const result = parseRawCodexConfig(...args)
+  matcherSets.push(result.matchers)
+  return result
+}

 describe('parseCodexConfig', () => {
  it('honors only the five bridge-supported Codex events, dropping the rest', () => {
@@ -62,8 +71,9 @@ describe('parseCodexConfig', () => {
  })

  it('keeps a valid Rust-regex matcher when present', () => {
-    const { config } = parseCodexConfig({ PreToolUse: [{ matcher: '(?i)^bash$', hooks: [{ type: 'command', command: 'b.sh' }] }] })
+    const { config, matchers } = parseCodexConfig({ PreToolUse: [{ matcher: '(?i)^bash$', hooks: [{ type: 'command', command: 'b.sh' }] }] })
    expect(config.PreToolUse![0]!.matcher).toBe('(?i)^bash$')
+    expect(matchers.matches('(?i)^bash$', 'BASH')).toBe(true)
  })

  it('rejects an invalid regex matcher with its event name', () => {
--- a/packages/hooks/hooks-codex/tests/matcher-lifecycle.spec.ts
+++ b/packages/hooks/hooks-codex/tests/matcher-lifecycle.spec.ts
@@ -9,6 +9,7 @@ import LocalSubprocessService from '@deepseek-ai/dsh-subprocess-local'
 const matcherLifecycle = vi.hoisted(() => {
  const registry = {
    matches: vi.fn(() => true),
+    diagnostic: vi.fn<(matcher: string | undefined) => string | undefined>(() => undefined),
    dispose: vi.fn<() => void>(),
  }
  return {
@@ -26,9 +27,30 @@ const dirs: string[] = []
 afterEach(() => {
  for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true })
  vi.clearAllMocks()
+  matcherLifecycle.registry.diagnostic.mockReturnValue(undefined)
 })

 describe('hooks-codex matcher lifecycle', () => {
+  it('disposes the compiled set when one event-specific diagnostic rejects the config', async () => {
+    const { parseCodexConfig } = await import('@deepseek-ai/dsh-hooks-codex/src/config.ts')
+    matcherLifecycle.registry.diagnostic.mockImplementation((matcher: string | undefined) => (
+      matcher === '[' ? 'invalid codex regex matcher "["' : undefined
+    ))
+
+    expect(() => parseCodexConfig({
+      PreToolUse: [
+        { matcher: '(?i)^bash$', hooks: [{ type: 'command', command: 'first' }] },
+        { matcher: '[', hooks: [{ type: 'command', command: 'second' }] },
+      ],
+    })).toThrow('invalid codex regex matcher "[" on event "PreToolUse"')
+
+    expect(matcherLifecycle.compileMatchers).toHaveBeenCalledExactlyOnceWith(
+      new Set(['(?i)^bash$', '[']),
+      'codex',
+    )
+    expect(matcherLifecycle.registry.dispose).toHaveBeenCalledOnce()
+  })
+
  it('gives the loaded config one matcher registry and disposes it on plugin teardown', async () => {
    const dir = mkdtempSync(join(tmpdir(), 'dsh-hooks-codex-matchers-'))
    dirs.push(dir)
@@ -44,10 +66,10 @@ describe('hooks-codex matcher lifecycle', () => {
    await ctx.plugin(LocalBashExecutor, { timeoutMs: 10_000 })
    const fiber = await ctx.plugin(HooksCodex, { configPath, model: 'm' })

-    expect(matcherLifecycle.compileMatchers).toHaveBeenCalledExactlyOnceWith([
+    expect(matcherLifecycle.compileMatchers).toHaveBeenCalledExactlyOnceWith(new Set([
      '(?i)^bash$',
-      '(?i)^bash$',
-    ], 'codex')
+    ]), 'codex')
+    expect(matcherLifecycle.registry.diagnostic).toHaveBeenCalledTimes(2)
    expect(matcherLifecycle.registry.dispose).not.toHaveBeenCalled()

    await fiber.dispose()