fix(codex): accept any JSON tool_input/tool_response on Codex tool hooks (#188 follow-up) - #404
Merged
Merged
Conversation
…oks (#188 follow-up) The pinned rust-v0.147.0 pre-tool-use/post-tool-use input schemas declare `"tool_input": true` and `"tool_response": true`, so Codex may deliver scalar or null payloads. The generated Codex hook wrapper and the event-route envelope validator now require presence only for Codex and keep the documented object requirement for Claude. Addresses the PR #378 Codex review thread.
🦋 Changeset detectedLatest commit: 8eb073e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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. |
commit: |
ScriptedAlchemy
added a commit
that referenced
this pull request
Sep 3, 2026
ScriptedAlchemy
added a commit
that referenced
this pull request
Sep 3, 2026
ScriptedAlchemy
added a commit
that referenced
this pull request
Sep 3, 2026
ScriptedAlchemy
added a commit
that referenced
this pull request
Sep 3, 2026
25 tasks
ScriptedAlchemy
added a commit
that referenced
this pull request
Sep 3, 2026
ScriptedAlchemy
added a commit
that referenced
this pull request
Sep 3, 2026
ScriptedAlchemy
added a commit
that referenced
this pull request
Sep 3, 2026
ScriptedAlchemy
added a commit
that referenced
this pull request
Sep 3, 2026
ScriptedAlchemy
added a commit
that referenced
this pull request
Sep 3, 2026
ScriptedAlchemy
added a commit
that referenced
this pull request
Sep 3, 2026
ScriptedAlchemy
added a commit
that referenced
this pull request
Sep 3, 2026
/#379/#385/#390) (#408) * fix: address late review threads on merged PRs #368/#373/#374/#377/#378/#379/#385/#390 - dev: lease the adopted epoch in EpochAdoptionPolicy until replaced or closed; select the contract-matrix target from the server's own target list; apply the session timeout per matrix request; observe lifecycle progress through the session trace via a new ContractMatrixClient.observeProgress seam (#385) - playground: wait for a hard-link catalog publisher to release its staging link before adopting the sidecar; return to discovery when the publication is rolled back (#377) - build: run the Agent Plugins byte lane over portable/ during ordinary artifact validation; reject every forbidden control character in header values (#373) - events/hooks: Codex PostToolUse accepts any present JSON tool_response (#378) - api: project only contract fields of adapter capability rows in inspect (#390) - tests/support: digest the real Claude home in the live session guard; isolate USERPROFILE alongside HOME (#374) - docs: Claude plugin-root cwd exception, parked-pin trigger independence, provider typing contract, portable validation moments (#368/#379/#382/#373) * fix(dev): lease before publishing contract status; align fixtures with the portable byte lane - EpochAdoptionPolicy acquires the epoch lease before publishing a passed dev.contract.status and announces adoption synchronously with it, so a status reader never sees "passed" for an epoch that is not yet adopted - hooks.test: Codex PostToolUse accepts a string tool_response and rejects a missing one; Claude keeps the object check - mcp-session-service/public-api-packed fixtures: Agent Plugins forbids placeholders in headers and non-bare/non-./ commands, and ordinary artifact validation now enforces the standard, so the fixtures carry a literal header and a bare `node` command * chore(changeset): drop the Codex tool_response bullet already released by #404 * fix(playground,test): withdraw a failed catalog publication before releasing its staging link; bind custom observeProgress - #persistSnapshot rolls the sidecar back while the staging link still exists when the post-link directory fsync fails, so a concurrent reader keeps seeing an in-progress publication until the path is withdrawn instead of adopting a briefly singly linked file - contractProgressObserver invokes a client's observeProgress method with the client as receiver * chore(changeset): one-paragraph summary ending with the PR reference * fix(playground): recover a catalog staging link abandoned by an exited publisher After the settle deadline, a matching .stage-<pid>-* link whose publisher pid no longer exists is an abandoned publication of an already fsynced sidecar: withdraw the orphan and adopt the sidecar instead of rejecting the epoch forever. A live publisher's staging link is still never yanked. Adds the @internal catalogStagingSettleDeadlineMs seam for deterministic tests. * fix(playground): fsync the catalog directory after withdrawing an abandoned staging link * fix(dev): forward request _meta (progress token) through McpSession and the dev matrix client Lifecycle fixtures pass their generated progressToken as params._meta; the session adapter and McpSession.callTool dropped it, so generated routes never enabled sendProgress and every progress-gated lifecycle fixture failed the dev matrix. McpSessionToolCallOptions and McpClient.callTool now carry _meta. * fix(dev,playground): restart the adoption drain after a handoff race; keep the staging link when a sidecar rollback fails - EpochAdoptionPolicy reschedules its drain from the completion handler when a candidate arrived between the loop's last empty check and #processing being cleared, and settled() waits through restarts - #persistSnapshot releases the staging link after a failed publication only once the owned sidecar is confirmed withdrawn, so a rollback failure never leaves a singly linked sidecar for readers to adopt * ci: retrigger checks for the rebased head * chore: drop the portable byte-lane changes superseded by #406; keep the _meta assertions * fix(playground): restore the staging guard when a recovery fsync fails * fix(playground): accept a concurrently restored staging guard (EEXIST aliasing the sidecar) during recovery * fix(dev): recheck supersession after the adoption lease settles * fix(playground): keep a fresh pid-owned guard when recovery can neither re-link nor withdraw the sidecar * fix(playground): fsync every compensating recovery guard before trusting it
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
1 participant
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.
Summary
post-tool-use.command.input.schema.json: the pinned rust-v0.147.0 pre-tool-use/post-tool-use input schemas declare"tool_input": trueand"tool_response": true(any JSON value), but the generated Codex hook wrapper and the event-route envelope validator still required objects, so a valid scalar/nulltool_response(ortool_input) failed before reaching the handler.validateNativeInput(generated wrapper) andvalidateNativeEventEnvelope(events/projection.ts) now require presence only (tool_input is required/tool_response is required), matching the existing PermissionRequesttool_inputhandling. Claude keeps its documented object requirement and error text.hooks.test.tsruns the built CodexPostToolUsewrapper with string/number/boolean/nulltool_responseand a stringtool_input(handler receives them, exit 0); the malformed-input matrix now asserts the Codex presence error vs. the Claude object error per target.event-project.test.tscovers the envelope validator for every JSON kind ontool/beforeandtool/after.Refs #188.
Test plan
pnpm build,pnpm typecheck,pnpm lintrstestunit:event-project,codex-hook-contract,lifecycle-replay-service,lifecycle-replay-routesrstestintegration:hooks.test.ts(26/26)