Skip to content

Project Codex hook notifications into agent events#70969

Merged
pashpashpash merged 3 commits into
mainfrom
codex/app-server-notification-projection
Apr 24, 2026
Merged

Project Codex hook notifications into agent events#70969
pashpashpash merged 3 commits into
mainfrom
codex/app-server-notification-projection

Conversation

@pashpashpash

Copy link
Copy Markdown
Contributor

This is the next small piece of Codex app-server notification projection.

Most of the adapter-level parity work is already present on main now: Codex mode emits llm_input, llm_output, and agent_end, mirrors transcript writes through before_message_write, projects item lifecycle events, carries token usage, and fires compaction hooks as notification-level observations.

One app-server notification family was still being ignored: Codex native hook lifecycle notifications. Codex now emits hook/started and hook/completed events for its own native hooks. Those are not OpenClaw plugin hooks and should not be treated as if they are, but they are useful runtime evidence for trajectories and debugging.

This change projects those notifications into codex_app_server.hook agent events with normalized metadata such as the hook run id, event name, handler type, source, status, duration, and output entries. That lets OpenClaw observe the native Codex hook lifecycle without pretending it owns or can rewrite that lifecycle.

I also added the focused projector coverage for started and completed hook notifications, and updated the Codex harness docs to state the boundary clearly: these notifications become debug/trajectory agent events, not OpenClaw plugin hook invocations.

@aisle-research-bot

aisle-research-bot Bot commented Apr 24, 2026

Copy link
Copy Markdown

🔒 Aisle Security Analysis

We found 1 potential security issue(s) in this PR:

# Severity Title
1 🟡 Medium Unbounded hook output forwarded into agent events can cause memory/CPU/network DoS
1. 🟡 Unbounded hook output forwarded into agent events can cause memory/CPU/network DoS
Property Value
Severity Medium
CWE CWE-400
Location extensions/codex/src/app-server/event-projector.ts:414-440

Description

handleHookNotification() reads run.entries from hook notifications and forwards the resulting array (including each entry's text) into an emitted agent event without any size limits.

This is risky because hook output can be influenced by:

  • Repo/project hooks (e.g., .codex/hooks.json), which may be controlled by an untrusted repository opened by a user.
  • Hook commands that can print arbitrarily large stdout/stderr or emit extremely many entries.

As a result, a malicious (or buggy) hook can cause:

  • Memory pressure (large arrays/strings retained in event objects)
  • CPU overhead (parsing/flattening large arrays)
  • Excessive logging/telemetry/network traffic in downstream consumers of onAgentEvent

Vulnerable code:

const entries = readHookOutputEntries(run.entries);
...
...(entries.length > 0 ? { entries } : {}),

And readHookOutputEntries() collects all valid entries and preserves the full text string for each.

Recommendation

Apply defensive caps/truncation before emitting hook output into agent events.

Suggested approach:

  • Cap number of entries (e.g., MAX_ENTRIES = 50)
  • Cap each entry text length (e.g., MAX_ENTRY_TEXT = 4_096)
  • Optionally cap statusMessage length as well
  • Include metadata indicating truncation occurred

Example:

const MAX_ENTRIES = 50;
const MAX_ENTRY_TEXT = 4096;
const MAX_STATUS_MESSAGE = 2048;

function truncate(s: string, max: number): string {
  return s.length > max ? s.slice(0, max) + "…" : s;
}

const rawEntries = readHookOutputEntries(run.entries);
const entries = rawEntries.slice(0, MAX_ENTRIES).map(e => ({
  ...("kind" in e ? { kind: e.kind } : {}),
  text: truncate(e.text, MAX_ENTRY_TEXT),
}));

const statusMessage = readNullableString(run, "statusMessage");
const safeStatusMessage =
  typeof statusMessage === "string" ? truncate(statusMessage, MAX_STATUS_MESSAGE) : statusMessage;

this.emitAgentEvent({
  stream: "codex_app_server.hook",
  data: {
    ...,
    statusMessage: safeStatusMessage,
    entries,
    hookOutputTruncated: rawEntries.length > MAX_ENTRIES,
  },
});

This reduces the risk of IDE/extension instability and oversized telemetry payloads when opening untrusted projects.


Analyzed PR: #70969 at commit 707a707

Last updated on: 2026-04-24T06:39:27Z

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation extensions: codex size: S maintainer Maintainer-authored PR labels Apr 24, 2026
@greptile-apps

greptile-apps Bot commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR projects Codex native hook/started and hook/completed app-server notifications into codex_app_server.hook agent events, adding handleHookNotification, two small helpers (readNumberLike, readHookOutputEntries), focused test coverage, and a doc clarification that these events are trajectory/debug observations and do not invoke OpenClaw plugin hooks.

The implementation is clean and consistent with the existing projector patterns. The only minor inconsistency is that readNumberLike adds a string-coercion branch for durationMs that differs from the readNumber helper used everywhere else in the file — worth a comment or substitution if string encoding isn't actually observed in the protocol.

Confidence Score: 5/5

Safe to merge — all findings are P2 style suggestions with no correctness impact.

The change is well-scoped, turn-scoping is correctly inherited from the existing gate, the new handler and helpers are logically sound, and test coverage is solid. The single comment is a minor style inconsistency that does not affect runtime behavior.

No files require special attention.

Prompt To Fix All With AI
This is a comment left during a code review.
Path: extensions/codex/src/app-server/event-projector.ts
Line: 661-669

Comment:
**`readNumberLike` is more permissive than needed**

`durationMs` in the Codex hook protocol is always a `number` when present, yet `readNumberLike` adds a string-coercion branch that silently converts `"42"``42`. The existing `readNumber` helper already handles the numeric case correctly and is used everywhere else in this file. If string coercion is genuinely required (e.g., because the Codex server can serialize this field as a string), a comment explaining that would help; otherwise `readNumber` keeps the codebase consistent and avoids masking unexpected type mismatches.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "project codex hook notifications" | Re-trigger Greptile

Comment thread extensions/codex/src/app-server/event-projector.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 756c44dde8

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread extensions/codex/src/app-server/event-projector.ts
@pashpashpash
pashpashpash merged commit 41c5ffc into main Apr 24, 2026
67 checks passed
@pashpashpash
pashpashpash deleted the codex/app-server-notification-projection branch April 24, 2026 06:43
zhongmairen pushed a commit to agencyos-cn/openclaw-bad that referenced this pull request Apr 24, 2026
* 'main' of https://github.com/openclaw/openclaw: (521 commits)
  perf: slim reset model selection imports
  perf: slim directive parse test imports
  ci(release): clarify npm telegram approval label
  test: relax live tail readiness checks
  ci(release): use release approval for npm telegram e2e
  Project Codex hook notifications into agent events (openclaw#70969)
  fix: match codex verbose tool logs to pi (openclaw#70966)
  feat(diagnostics): attach trace context to otel logs (openclaw#70961)
  docs(faq): tighten FAQ stub pointers and Related cards
  docs(help): rewrite help index to match new tab structure, drop redundant troubleshooting H1
  refactor(release): distill npm telegram docker runner
  ci(release): gate npm telegram e2e by release team
  ci(release): harden npm telegram beta e2e
  test(release): address npm telegram e2e review
  test(release): avoid docker argv secret values
  ci(release): add manual npm telegram beta e2e
  test(release): support convex npm telegram credentials
  docs: clarify private ws node setup
  fix: filter gateway node list locally
  feat(plugins): move Bonjour discovery into bundled plugin
  ...
Angfr95 pushed a commit to Angfr95/openclaw that referenced this pull request Apr 25, 2026
* project codex hook notifications

* keep codex hook duration strict

* include thread scoped codex hook notifications
ogt-redknie pushed a commit to ogt-redknie/OPENX that referenced this pull request May 2, 2026
* project codex hook notifications

* keep codex hook duration strict

* include thread scoped codex hook notifications
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 9, 2026
* project codex hook notifications

* keep codex hook duration strict

* include thread scoped codex hook notifications
globalcaos pushed a commit to globalcaos/tinkerclaw that referenced this pull request May 13, 2026
* project codex hook notifications

* keep codex hook duration strict

* include thread scoped codex hook notifications
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 24, 2026
* project codex hook notifications

* keep codex hook duration strict

* include thread scoped codex hook notifications
jameslcowan pushed a commit to jameslcowan/openclaw that referenced this pull request Jun 2, 2026
* project codex hook notifications

* keep codex hook duration strict

* include thread scoped codex hook notifications
sablehead pushed a commit to sablehead/openclaw that referenced this pull request Jun 10, 2026
* project codex hook notifications

* keep codex hook duration strict

* include thread scoped codex hook notifications
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Improvements or additions to documentation extensions: codex maintainer Maintainer-authored PR size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant