Repository navigation
fix: ignore timeout/statusMessage on Codex prompt hooks - #1752
Merged
Merged
Conversation
lcm doctor validated timeout and statusMessage on every Codex hook handler type, so a prompt or agent handler with e.g. timeout -1 or timeout 1.5 made every LCM hook row report hooks-file-unreadable. Codex defines those two variants as empty and its handler enum does not deny unknown fields, so it ignores both members and loads the file. validHandler now returns valid for prompt and agent right after the known-type check. command and mcp_tool keep their integer-token timeout and string statusMessage checks. The timeout ternary and the non-command guard became unreachable and are removed to keep the 100% branch gate. Gotcha: role detection still reads the command member of a prompt or agent handler; that pre-existing behaviour is left alone. Closes #1719 Signed-off-by: Bernardo Donadio <bcdonadio@bcdonadio.com>
bcdonadio
enabled auto-merge
October 25, 2026 06:14
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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.
Why
lcm doctorreported every LCM hook ashooks-file-unreadablewhenhooks.jsonheld apromptoragenthandler with a badtimeout(for example-1or1.5) or a non-stringstatusMessage. Codex 0.163.0 defines those two handler variants as empty and itsHookHandlerConfigenum does not deny unknown fields, so it ignores both members and loads the file. Doctor was stricter than Codex and told users their working hooks were broken.Before:
{"type":"prompt","timeout":-1}next to the LCM hooks giveshooks-file-unreadableon every row. After: the rows keep their real status (readyhere).How
In
validHandler(src/connectors/codex-readiness.ts), apromptoragenthandler now returns valid right after the known-type check, before anytimeoutorstatusMessagecheck.commandandmcp_toolkeep the existing integer-tokentimeoutand stringstatusMessagechecks unchanged. The now-unreachable non-command guard and the timeout ternary are removed so no dead branches remain.Role detection for prompt and agent handlers that carry a
commandstring is a separate pre-existing behaviour and is not touched.Closes #1719.
Testing
test/connectors/codex-readiness.test.ts: the tworejectedprompt/agent timeout cases are nowaccepted, with added accepted cases for oddtimeoutandstatusMessagevalues on prompt/agent, and rejection cases for number/boolean/objectstatusMessageoncommandandmcp_tool(null stays accepted). The flipped case fails on the unfixed source ({"type":"prompt","timeout":-1}: expectedready, gothooks-file-unreadable).test/codecov-config.test.tspasses with the new ownership fragment.tsc --noEmitand eslint on the changed files are clean;codex-readiness.tshas 100% statement, branch, function and line coverage.Reviewer guide
Start with the
validHandlerdiff, then the flipped assertions in therejects a timeout or additionalContextLimit ...test. Thedocs/vscode-codex.mdrow now scopes the whole-number timeout rule to command and mcp_tool handlers. A patch changeset and atest/codecov-ownershipfragment (nocodecov.ymlchange) are included.