fix(hooks): pass additionalContextLimit through for Codex CLI - #2576
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
Part of #2496 — two judgment-free slices from that issue. The rest of #2496 stays open: it turns on design decisions (gateway ownership for
[agents]/[[skills.config]]/project_doc_*/mcp_oauth_callback_*, and the Codex plugin bundle target).Slice A — stale comment on the codexcli permissions override
The doc comment on
CodexcliPermissionsOverrideSchema(src/types/permissions.ts) said the override is "looseObject(verbatim passthrough) so future top-level Codex config keys can be authored without Rulesync modeling each one". That has not been true since the override gained an allowlist:computeCodexcliOverridePatch(src/features/permissions/codexcli-permissions.ts) emits only the five keys inCODEXCLI_OVERRIDE_KEYS(src/constants/codexcli-paths.ts) and skips anything else with a warning. A reader following the old comment would author a new top-level Codex key and find it silently dropped.The comment now describes the allowlist, names the five keys and where they live, and explains what
looseObjectactually buys (an unmodeled key parses and is then reported, rather than failing validation outright) and how to add a key. No behavior change.Slice B —
additionalContextLimitpassthrough for Codex CLICodex CLI documents a per-handler
additionalContextLimiton.codex/hooks.jsonhooks: the token threshold above which Codex writes the hook's additional context to a file and passes that path instead of the text, defaulting to 2500 (https://learn.chatgpt.com/docs/hooks). Rulesync had no way to express it, so authoring it was impossible and an existing config carrying it lost the field on import.The shared hooks converter had boolean, string, string-array and group passthrough kinds but no number kind, so this adds
numberPassthroughFieldsmirroringbooleanPassthroughFields:src/features/hooks/tool-hooks-converter.ts— the config field plusemitNumberPassthroughFields/importNumberPassthroughFields. The guard isNumber.isFinite, soNaN/Infinity(which JSON cannot represent) and a numeric string are both ignored in either direction rather than leaking into a config Codex would reject.src/types/hooks.ts— canonicaladditionalContextLimit: z.optional(z.number()).src/features/hooks/codexcli-hooks.ts— registered inCODEXCLI_CONVERTER_CONFIGunder the same name on both sides.docs/reference/file-formats.md— documented alongside the other per-hook fields.No hooks JSON schema regeneration was needed:
pnpm run generate:schemaemits config / mcp / permissions schemas only, andhooks.jsonchas no published schema. Running it produced no diff.Testing
additionalContextLimitbeing ignored on import.npx vitest run --config vitest.e2e.config.ts src/e2e/e2e-hooks.spec.ts— 75 passed.pnpm cicheck— all green.🤖 Generated with Claude Code