mirror of
https://github.com/deepseek-ai/deepseek-harness
synced 2026-08-15 21:04:50 +00:00
fix(hooks): reuse compiled Codex matchers
This commit is contained in:
@@ -25,10 +25,10 @@ import type { PostToolDecision, PreToolDecision, ToolExecution, ToolExecutionRes
|
||||
import {
|
||||
appendHookInvoked,
|
||||
appendHookResult,
|
||||
compileMatchers,
|
||||
createDetachedRuns,
|
||||
DEFAULT_HOOK_TIMEOUT_MS,
|
||||
DEFAULT_STDERR_SUMMARY_MAX_CHARS,
|
||||
matchesMatcher,
|
||||
mergeHookOutputs,
|
||||
runHook,
|
||||
type HookOutput,
|
||||
@@ -99,11 +99,26 @@ export function apply(ctx: Context, config: Config): void {
|
||||
|
||||
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',
|
||||
)
|
||||
|
||||
// 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
|
||||
// continuation (docs/defensive-patterns.md: dispose must reach quiescence).
|
||||
// continuation before releasing matchers (docs/defensive-patterns.md:
|
||||
// dispose must reach quiescence).
|
||||
const detached = createDetachedRuns()
|
||||
ctx.effect(() => () => detached.drain(), 'hooks-codex: drain detached hook runs')
|
||||
ctx.effect(() => async () => {
|
||||
try {
|
||||
await detached.drain()
|
||||
} finally {
|
||||
matchers.dispose()
|
||||
}
|
||||
}, 'hooks-codex: drain detached hook runs and dispose matchers')
|
||||
|
||||
/**
|
||||
* Run and fold one configured Codex hook point.
|
||||
@@ -127,9 +142,11 @@ export function apply(ctx: Context, config: Config): void {
|
||||
// Run hooks in the agent's session workspace so relative paths address the
|
||||
// user's project rather than the server launch directory.
|
||||
const workdir = opts.agent?.session.header.cwd
|
||||
// Keep each dialect's audit stamping readable beside its payload mapping.
|
||||
/* jscpd:ignore-start */
|
||||
for (const group of groups) {
|
||||
// The protocol library owns Codex's exact-literal/Rust-regex split.
|
||||
if (!matchesMatcher(group.matcher, matchQuery, 'codex')) continue
|
||||
if (!matchers.matches(group.matcher, matchQuery)) continue
|
||||
for (const hook of group.hooks) {
|
||||
const handlerId = nextHandlerId(point)
|
||||
const session = opts.agent?.session
|
||||
@@ -139,6 +156,7 @@ export function apply(ctx: Context, config: Config): void {
|
||||
...group.matcher !== undefined ? { matcher: group.matcher } : {},
|
||||
})
|
||||
}
|
||||
/* jscpd:ignore-end */
|
||||
const { output, durationMs } = await runHook(ctx.bash, hook, {
|
||||
payload,
|
||||
defaultTimeoutMs,
|
||||
|
||||
73
packages/hooks/hooks-codex/tests/matcher-lifecycle.spec.ts
Normal file
73
packages/hooks/hooks-codex/tests/matcher-lifecycle.spec.ts
Normal file
@@ -0,0 +1,73 @@
|
||||
import { createRequire } from 'node:module'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { Context } from 'cordis'
|
||||
import { LocalBashExecutor } from '@deepseek-ai/dsh-bash-local'
|
||||
import LocalSubprocessService from '@deepseek-ai/dsh-subprocess-local'
|
||||
|
||||
interface RustRegexInstance {
|
||||
free(): void
|
||||
}
|
||||
|
||||
const dirs: string[] = []
|
||||
afterEach(() => { for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true }) })
|
||||
|
||||
describe('hooks-codex matcher lifecycle', () => {
|
||||
it('constructs one reusable runtime regex and frees it on plugin teardown', async () => {
|
||||
// The product deliberately loads rregex through createRequire so Cordis can
|
||||
// discover the bridge synchronously. Patch that SAME CJS export, rather
|
||||
// than an ESM mock that would not observe the production load path.
|
||||
const require = createRequire(new URL('../../hook-protocol/package.json', import.meta.url))
|
||||
const rregex = require('rregex') as { RRegex: new(pattern: string) => RustRegexInstance }
|
||||
const OriginalRRegex = rregex.RRegex
|
||||
const construct = vi.fn<(pattern: string) => void>()
|
||||
const free = vi.fn<() => void>()
|
||||
|
||||
class CountingRRegex extends OriginalRRegex {
|
||||
constructor(pattern: string) {
|
||||
super(pattern)
|
||||
construct(pattern)
|
||||
}
|
||||
|
||||
override free(): void {
|
||||
free()
|
||||
super.free()
|
||||
}
|
||||
}
|
||||
|
||||
const dir = mkdtempSync(join(tmpdir(), 'dsh-hooks-codex-matchers-'))
|
||||
dirs.push(dir)
|
||||
const configPath = join(dir, 'hooks.json')
|
||||
writeFileSync(configPath, JSON.stringify({ hooks: {
|
||||
PreToolUse: [{ matcher: '(?i)^bash$', hooks: [{ type: 'command', command: 'true' }] }],
|
||||
PostToolUse: [{ matcher: '(?i)^bash$', hooks: [{ type: 'command', command: 'true' }] }],
|
||||
} }))
|
||||
|
||||
rregex.RRegex = CountingRRegex
|
||||
vi.resetModules()
|
||||
try {
|
||||
const HooksCodex = await import('@deepseek-ai/dsh-hooks-codex')
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(LocalSubprocessService)
|
||||
await ctx.plugin(LocalBashExecutor, { timeoutMs: 10_000 })
|
||||
const fiber = await ctx.plugin(HooksCodex, { configPath, model: 'm' })
|
||||
|
||||
// The parser validates both groups one-shot (2 construct/free pairs), then
|
||||
// the runtime registry compiles the duplicate pattern only once and owns it.
|
||||
expect(construct.mock.calls.map(([pattern]) => pattern)).toEqual([
|
||||
'(?i)^bash$',
|
||||
'(?i)^bash$',
|
||||
'(?i)^bash$',
|
||||
])
|
||||
expect(free).toHaveBeenCalledTimes(2)
|
||||
|
||||
await fiber.dispose()
|
||||
expect(free).toHaveBeenCalledTimes(3)
|
||||
} finally {
|
||||
rregex.RRegex = OriginalRRegex
|
||||
vi.resetModules()
|
||||
}
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user