Skip to content

fix(hook): check the files a Codex apply_patch changes; init offers an AGENTS.md section - #182

Merged
dgesteves merged 2 commits into
mainfrom
fix/os2-38-codex-apply-patch-hook
Oct 16, 2026
Merged

dgesteves merged 2 commits into
mainfrom
fix/os2-38-codex-apply-patch-hook

Conversation

@dgesteves

@dgesteves dgesteves commented Oct 16, 2026 •

Copy link
Copy Markdown
Owner

Roadmap item: OS2-38 (wave 2: the Codex apply_patch fix and the opt-in AGENTS.md section in init).

What was wrong

Codex installs the Claude Code plugin from the same marketplace and runs its hooks/hooks.json. Its apply_patch matches our Write|Edit matcher (Codex aliases them), but the hook input carries the patch in tool_input.command, and check-ui.mjs read only tool_input.file_path, so it exited 0 with no output on every Codex edit. In Codex's source, core/src/tools/handlers/apply_patch.rs builds the PostToolUse input as {"command": <patch>}, and hooks/src/engine/dispatcher.rs matches Edit and Write to apply_patch.

Repro on examples/shadcn-demo with the built CLI (ONSYSTEM_BIN=dist/cli.js) and a Codex-shaped payload (tool_name: "apply_patch", tool_input.command = an *** Update File: patch that turns variant="destructive" into variant="danger" on line 18 of app/settings/danger-zone.tsx):

  • main: exit 0, no stdout, no stderr.
  • this branch: exit 2, with
    app/settings/danger-zone.tsx: this change breaks the project's design system (onsystem check). ...
    18:25 error [no-unknown-variant] "danger" is not a valid variant for <Button>. ...
    (10 other findings elsewhere in the file predate this edit; leave them unless asked.)
    

What changed

Hook (plugins/onsystem/hooks/check-ui.mjs)

  • For apply_patch, it reads the files from the patch's *** Add File:, *** Update File: and *** Move to: lines. A deleted file is skipped, and a moved file is checked at its new path.
  • An added file is checked whole, as a Write is. An updated file is held to the lines its hunks change, as an Edit is: each hunk's context and - lines are the old text, and its context and + lines are the new text. So a Codex edit is not blamed for older findings in the file. When a hunk's new text appears more than once in the file, the findings go back as a non-blocking note, as for an Edit.
  • A patch that touches several UI files runs check once per project with all of them, and the results are matched back to each file. A single-file run behaves as before, byte for byte (the existing hook tests pass unchanged).
  • It also reads tool_input.path and tool_input.filePath, as the brief asked, but only when the input carries the text the tool wrote (new text, content or file_text). VS Code ignores hook matchers and runs hooks after every tool, and its read_file names the file filePath. Without this condition, reading a .tsx file there would run the check and block. oldString/newString and old_str/new_str count as an Edit's old and new text. A code field is not read as the file: VS Code's insert_edit_into_file sends a snippet with ...existing code... lines, and onsystem's own check_ui sends unsaved code with a path, so both stay silent.
  • A hunk that only deletes lines changes no line of its own, so it is not used to place the file's edits. Its context may appear more than once in the file, and it no longer turns the errors of the other hunks into a non-blocking note.
  • hooks.json is unchanged: Codex's matcher aliases already route apply_patch to Write|Edit, and a test pins the matcher.

init (src/init.ts)

In a terminal, after the baseline question, init offers (default yes) to add a marked section to the root's AGENTS.md and/or CLAUDE.md. It does nothing when neither file exists. The section:

<!-- BEGIN:onsystem-agent-rules -->
# Design system

This project's UI is checked against its design system by onsystem.

- Before writing UI, look up its components, props, variants and tokens with the onsystem tools (`list_components`, `get_component`, `get_tokens`).
- After changing a `.tsx` or `.jsx` file, run `check_ui` on it and fix every error. Without the onsystem tools, run `npx -y onsystem check <file>` instead.
<!-- END:onsystem-agent-rules -->

A later run replaces the section between the markers. An END marker is paired with the nearest BEGIN before it, so a BEGIN whose END was deleted is not treated as a section, and the text after it is kept. If the section is already current, nothing is asked. A CLAUDE.md that imports AGENTS.md (an @AGENTS.md line) is left alone. Next's own block in AGENTS.md is kept as is. --yes and runs with no terminal never write it. The brief allowed a flag to write the section with --yes, but that flag would live in src/cli.ts, which is outside this item's files, so it can follow separately if wanted. create-next-app 16.4.0 writes AGENTS.md only: I checked its published dist/index.js, which writes AGENTS.md and never mentions CLAUDE.md.

Docs: docs/clients.md (intro, the "Plugins for other agents" lead, the Codex entry with the one-time /hooks trust step, and the closing paragraph on the init section), docs/plugin.md (a "Codex runs it too" bullet) and one README sentence. Changeset: patch.

Not in this PR

  • Hooks for Cursor, Copilot CLI, VS Code and Gemini CLI move to the public roadmap. Each needs its own output format and testing.
  • plugins/onsystem/README.md and SKILL.md still describe the hook as Claude Code's only. Both are outside this item's files, so they are a follow-up.
  • This was not run in a live Codex session, which would need model calls. It was tested with synthetic payloads shaped like Codex's source and docs. Codex's hook docs say PostToolUse accepts exit 2 with the reason on stderr, and that plugin hooks run only after the user trusts them in /hooks.

How it was tested

New tests:

  • test/hook-apply-patch.test.ts has 9 tests. 7 of them fail on main. Two pass there because main is silent: the one that checks a patch with no UI files, and the one that checks a code snippet. They cover:
    • Update File with an absolute and a relative path, held to the hunk's lines
    • Add File, and Move to
    • several files in one run, with Markdown and deleted files left out
    • an ambiguous hunk, which becomes a note
    • a deletion-only hunk with repeated context next to a placed hunk, which still blocks on the placed hunk's error only
    • path/filePath writes, and read_file/view, which stay silent
    • a { filePath, code } snippet (insert_edit_into_file) and a check_ui call with code and path, on files with older findings, which stay silent
    • matching each file's findings in the real CLI's JSON for a two-file run
  • test/init-agents-md.test.ts has 4 tests, and all 4 fail on main. They cover:
    • appending to a Next-style AGENTS.md, and leaving a CLAUDE.md that imports it alone
    • a no-op when the section is current, and an in-place update when it is old
    • writing both files when CLAUDE.md does not import AGENTS.md
    • a BEGIN marker whose END was deleted: the user's text after it survives two runs, and an older section after it is the one updated
    • declining, --yes, no terminal, and no file

Gate, all passing:

  • pnpm install --frozen-lockfile --prefer-offline
  • pnpm format:check
  • pnpm lint
  • pnpm typecheck
  • pnpm test: 78 files passed, 1 skipped; 1012 tests passed, 3 skipped
  • pnpm build: schema.json unchanged
  • pnpm smoke: smoke test passed
  • The Website job, as CI runs it after pnpm build: pnpm --filter site generate, pnpm --filter site test (16 files, 269 tests passed), pnpm --filter site typecheck, pnpm --filter site lint and pnpm --filter site build all pass. The built /docs/clients page, its .md and llms-full.txt carry the new init sentence, with no <!-- in it.

Also on examples/shadcn-demo with the built CLI: an insert_edit_into_file payload ({ explanation, filePath, code }) whose snippet holds only the <CardTitle> line of app/settings/danger-zone.tsx exits 0 with no output. The Codex repro above still exits 2 with the one 18:25 error.

…AGENTS.md section in init

Codex installs the Claude Code plugin and runs its hook on apply_patch, which
matches Write|Edit, but the patch is in tool_input.command and the hook read
only tool_input.file_path, so it exited 0 on every Codex edit. The hook now
reads the files a patch adds, updates or moves, checks each .tsx or .jsx file
in one run per project, and holds an updated file to the lines its hunks
change. It also reads path or filePath when the tool's input carries the text
it wrote.

init, in a terminal, offers to add a marked onsystem section to AGENTS.md or
CLAUDE.md, with the MCP and CLI forms, and updates it in place on later runs.

OS2-38
…arker, and skip deletion-only hunks

- The hook no longer treats a `code` field as a whole-file write. VS Code's
  insert_edit_into_file sends a snippet with `...existing code...` lines,
  and check_ui sends unsaved code, so such a call blocked on findings the
  edit did not cause. Only `content` and `file_text` count as the file.
- init pairs the agent-rules END marker with the nearest BEGIN before it,
  so a BEGIN whose END was deleted no longer makes a later run replace the
  user's text after it.
- A patch hunk that only deletes lines is not looked for when placing a
  file's edits, so its repeated context no longer turns the file's errors
  into a non-blocking note.
- docs/clients.md and the changeset name the markers without `<!--`,
  which the site keeps out of llms-full.txt.
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