Skip to content

Support Codex rule injection hooks#1

Merged
code-yeongyu merged 13 commits into
mainfrom
codex-post-hook-support
May 18, 2026
Merged

Support Codex rule injection hooks#1
code-yeongyu merged 13 commits into
mainfrom
codex-post-hook-support

Conversation

@code-yeongyu
Copy link
Copy Markdown
Owner

@code-yeongyu code-yeongyu commented May 18, 2026

Summary

  • Add Codex rule injection hook support.
  • Cache dynamic rule discovery and matcher work.
  • Harden malformed hook input and Windows path handling.

Improvement table

Area Baseline / risk before this PR Final pinned behavior Delta / proof
Codex hook support No Codex rule-injection hook surface. SessionStart, UserPromptSubmit, PostToolUse, and PostCompact hook paths covered. New behavior pinned by test/codex-hook.test.ts.
Dynamic duplicate targets Repeated targets could repeat discovery/read/match work. Duplicate-target benchmark and engine counters are tracked by scripts/bench-codex-rules.mjs. Baseline recorded: duplicate-targets median 5.555042ms, counters findProjectRoot:40, findCandidates:40, readFile:4800.
Distinct targets Distinct target fan-out was not benchmarked. Distinct-target benchmark records discovery/read counters. Baseline recorded: distinct-targets median 28.971417ms, counters findProjectRoot:40, findCandidates:3200, readFile:4800.
Repeated hook fast path Repeat PostToolUse output/work was not pinned. Benchmark compares repeat output bytes and median repeat duration against baseline. npm run bench passes and guards repeat-output regressions.

Verification

  • npm test
  • npm run check
  • npm run bench
  • npm pack --dry-run
  • no-excuse TypeScript gate

Review

  • Local verification completed for Codex plugin behavior.
  • Cubic review passed before merge.

Track Codex MCP filesystem PostToolUse paths and keep dynamic rule injection aligned with the plugin hook output contract.

Remove stale tracked-tool constants and duplicate apply_patch path scanning while adding contract, deduplication, and no-op tests.

Plan: plans/post-hook-support-codex-plugins.md
Pin plugin metadata, use Codex-managed plugin-root interpolation, cap recursive rule scans, and add package/scanner regression tests with Windows CI coverage.

Plan: ../plans/plugin-portability-hardening.md
Replace the external glob matcher with a small internal matcher and pin common recursive, negative, and brace glob behavior. Keep package smoke coverage pinned so clean Codex marketplace copies run without node_modules.

Plan: ../plans/plugin-portability-hardening.md
Git on Windows defaults to core.autocrlf=true and converts LF to CRLF
on checkout, which makes biome's --check fail for every TypeScript file
(33 errors observed on windows-latest). Add a .gitattributes that
forces LF on every checkout so the Windows CI matrix matches Linux/macOS.

Plan: ../plans/plugin-portability-hardening.md
On Windows path.relative() returns backslash-separated paths, so the
PostToolUse rule context emitted strings like 'src\app.ts' instead of
'src/app.ts'. Codex feeds additionalContext verbatim into the model
prompt, and the rest of the engine already canonicalizes to POSIX via
toPosixPath, so align displayPath with the same convention.

Pin the new shape in the existing PostToolUse test by adding a negative
assertion against the backslash form; the Windows CI matrix is the live
regression line.

Plan: ../plans/plugin-portability-hardening.md
Dynamic rule injection rebuilt glob regexes per candidate, called
`findProjectRoot` and the candidate scanner for every target file, and
re-read every rule file once per target. With 120 rules and 240 target
paths the hook spent ~746ms median, ran 9,600 root lookups, and read
files 1,152,000 times in the benchmark.

Behavior-preserving changes:
- Dedupe `targetPaths` before iteration and skip per-target
  `findProjectRoot` when the target lives under the cwd root.
- Share a single `RuleDiscoveryCache` across all target iterations so
  `scanRuleFiles` and `existsSync`/`statSync` for single-file sources
  each run once per directory.
- Add a per-run `loadedRuleContent` map keyed by `realPath` so the same
  rule file is parsed and hashed once per dynamic load.
- Cache `isCandidateWithinProject` decisions per candidate/projectRoot.
- Cache compiled positive/negative glob matchers per pattern set in
  the matcher module.

Pin the new behavior with unit tests for engine counter coverage,
finder source ordering, matcher pattern alternation, scanner symlink
loops, and a PostToolUse multi-target hook test that asserts the
matching rule is emitted exactly once.

Add `scripts/bench-codex-rules.mjs` plus an `npm run bench` entry that
exercises duplicate-target and distinct-target scenarios and supports
`--write-baseline`/`--compare` for counter-regression gating.

Benchmark vs 0910045 baseline (40 iterations, 5 warmups, 120 rules):

| scenario          | before                          | after                         |
| ----------------- | ------------------------------- | ----------------------------- |
| duplicate-targets | 746ms / 9600 roots / 1.15M reads | 4.8ms / 40 roots / 4800 reads |
| distinct-targets  | 227ms / 3200 roots / 384k reads  | 26ms / 40 roots / 4800 reads  |

Plan: ../../plans/codex-rules-matching-hook-performance.md
Guard hook stdin parsing before event dispatch so malformed or wrong-shape payloads no-op instead of surfacing JSON parser failures.

Plan: /Users/yeongyu/local-workspaces/codex-plugins/plans/posttooluse-hook-hardening.md
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 issues found across 50 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/rules/matcher.ts">

<violation number="1" location="src/rules/matcher.ts:130">
P1: Replacing `picomatch` with `globToRegExp` narrows supported glob syntax and can break existing rule patterns (e.g., character classes/extglob), causing unexpected rule match failures.</violation>
</file>

<file name="test/codex-hook.test.ts">

<violation number="1" location="test/codex-hook.test.ts:25">
P2: Use `fileURLToPath()` here instead of `.pathname`; the current value is not a portable file path.</violation>
</file>

<file name="scripts/bench-codex-rules.mjs">

<violation number="1" location="scripts/bench-codex-rules.mjs:109">
P2: Move target generation outside the timer so the benchmark measures `loadDynamicRules()` rather than path निर्माण overhead.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Re-trigger cubic

Comment thread src/rules/matcher.ts
return (path: string) => expression.test(path);
}

function globToRegExp(pattern: string): RegExp {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Replacing picomatch with globToRegExp narrows supported glob syntax and can break existing rule patterns (e.g., character classes/extglob), causing unexpected rule match failures.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/rules/matcher.ts, line 130:

<comment>Replacing `picomatch` with `globToRegExp` narrows supported glob syntax and can break existing rule patterns (e.g., character classes/extglob), causing unexpected rule match failures.</comment>

<file context>
@@ -83,6 +85,100 @@ function normalizePath(path: string): string {
+	return (path: string) => expression.test(path);
+}
+
+function globToRegExp(pattern: string): RegExp {
+	let source = "^";
+	for (let index = 0; index < pattern.length; index += 1) {
</file context>

Comment thread test/codex-hook.test.ts Outdated
Comment thread scripts/bench-codex-rules.mjs
code-yeongyu and others added 2 commits May 18, 2026 13:12
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 6 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/rules/engine.ts">

<violation number="1" location="src/rules/engine.ts:201">
P2: Dynamic match cache key omits frontmatter changes, which can return stale match decisions after rule metadata-only edits.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic

Comment thread src/rules/engine.ts
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 24 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="scripts/bench-codex-rules.mjs">

<violation number="1" location="scripts/bench-codex-rules.mjs:62">
P2: `hookFastPath` is never included in `--compare`, so regressions in the new benchmark path will not fail CI.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic

Comment thread scripts/bench-codex-rules.mjs
@code-yeongyu
Copy link
Copy Markdown
Owner Author

Status update: CI matrix, GitGuardian, and Cubic are all green for head ac04990. Normal merge is still blocked by base branch policy, and repository auto-merge is disabled. Please merge through the protected-branch flow or adjust the required policy condition; no admin bypass was used.

@code-yeongyu
Copy link
Copy Markdown
Owner Author

Status update: CI matrix, GitGuardian, and Cubic are green for head ac04990. Normal merge is still blocked by base branch policy and repository auto-merge is disabled. Please merge through the protected-branch flow or adjust the required policy condition. No admin bypass was used.

@code-yeongyu code-yeongyu merged commit 72dbad0 into main May 18, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant