Skip to content

feat(hooks): wire Codex notify from the manifest (codex.notify event)#40

Merged
vanducng merged 2 commits into
mainfrom
feat/codex-notify-hook
Jun 15, 2026
Merged

feat(hooks): wire Codex notify from the manifest (codex.notify event)#40
vanducng merged 2 commits into
mainfrom
feat/codex-notify-hook

Conversation

@vanducng

Copy link
Copy Markdown
Owner

Makes `vd install hooks` configure both agents from one manifest, fixing the source/copy drift where Claude ran the vd-installed copy while Codex ran the source script.

What

  • New `codex.notify` pseudo-event (parallels `statusLine`). A hook with `event = "codex.notify"` is copied into `/.claude/hooks/` like any other, but registered in `/.codex/config.toml` as the `notify` program.
  • Codex execs the program directly (no shell), so the notify uses the absolute hook path, not `$HOME`-literal.
  • Surgical `config.toml` edit (regex line splice) — preserves all other keys + comments byte-for-byte; backs up before writing; atomic. A replaced prior `notify` is reported (chain it via your notifier env, e.g. `CODEX_NOTIFY_FORWARD`).
  • `vd hooks uninstall` removes only our notify line.

Result

One managed file (`~/.claude/hooks/agent-notify.py`) serves both Claude (Stop/Notification) and Codex (notify) — no drift, and `vd install hooks` covers Codex on fresh machines.

Tests

New `codex_test.go` (preserve/replace/already-ours/missing-file/append/escaping/unwire/foreign/backup) + manifest + command tests. Live smoke: model + comment preserved, notify rewritten to abs path, settings.json untouched, chain-warning shown. `go test -race`, vet, lint all green.

A hook with event = "codex.notify" is installed into ~/.claude/hooks like any other, but registered in ~/.codex/config.toml as the notify program (absolute path — Codex execs directly). One managed file now serves both Claude (settings.json hooks) and Codex (notify), removing the source/copy drift. The config.toml edit is surgical (preserves other keys + comments, backs up); a replaced prior notify is reported so it can be chained via env.

@munmiu munmiu Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔍 OpenCodeReview found 6 issue(s) in this PR.

  • ✅ 6 posted as inline comment(s)
  • 📝 0 posted as summary

Comment thread internal/cli/install.go Outdated
Comment thread internal/cli/install.go Outdated
Comment thread internal/claudeconfig/codex.go Outdated
Comment thread internal/claudeconfig/codex.go
Comment thread internal/claudeconfig/codex.go
Comment thread internal/claudeconfig/codex.go
- handle multi-line notify arrays in the surgical regex (was single-line only)
- escape control characters (U+0000-001F) in TOML strings
- refuse >1 codex.notify hooks (Codex has one notify slot) instead of silent clobber
- use errors.Is(err, os.ErrNotExist) and wrap CodexConfigPath error
- tests for multi-line array preservation + control-char escaping
@vanducng vanducng merged commit 6c0276e into main Jun 15, 2026
2 checks passed
@vanducng vanducng deleted the feat/codex-notify-hook branch June 15, 2026 05:58
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