Repository navigation
fix(types): distinguish parse-only Zod function tools - #2517
Merged
Merged
Conversation
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. |
Contributor
Castiron custom code✅ No new custom-code files detected. 32 mixed files remain; 0 existing customizations changed. Compared 32 existing customizations unchanged
A changed generated baseline means this report cannot reliably identify which handwritten lines changed. Inspect the custom-code diffDownload the exact patch produced by this run (requires repository access): gh run download 33289705605 --repo openai/openai-node \
--name castiron-custom-code-33289705605-1 --dir /tmp/castiron-custom-code-33289705605-1
git apply --stat /tmp/castiron-custom-code-33289705605-1/custom-code.patch
cat /tmp/castiron-custom-code-33289705605-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin eea2292a4a523da9405161dde0a79ac5dc2ecb2a 64cc31d73a05821bba744e21553b6561616e4898
python3 scripts/castiron/custom_code_report.py report \
--base eea2292a4a523da9405161dde0a79ac5dc2ecb2a \
--head 64cc31d73a05821bba744e21553b6561616e4898 --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-64cc31d73a05
cat /tmp/castiron-custom-code-64cc31d73a05/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
Contributor
jbeckwith-oai
approved these changes
Aug 30, 2026
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
Contributor
There was a problem hiding this comment.
Reviewed the exact head. The overloads correctly distinguish guaranteed callbacks from parse-only or optional callbacks while retaining inferred Zod argument types. No blocking findings.
gh-actions-shared Bot
pushed a commit
to xf-qubit/openai-node
that referenced
this pull request
Sep 3, 2026
## Summary
- Pass the complete options metadata to `AutoParseableTool` and
`AutoParseableResponseTool` instead of passing the parsed argument value
type a second time.
- Preserve explicit parser-output, callback-argument, and function-name
types in both public custom-tool factories.
- Derive Chat Completions runnable metadata from the supplied callback:
a guaranteed callback is runnable, `undefined` is parse-only, and an
optional callback remains unguaranteed. Callback parameters are checked
against the parser's declared output.
## Reproduction
A custom factory call with metadata such as `{ name: 'lookup';
arguments: { city: string }; function: (args: { city: string }) =>
string }` currently returns a tool whose `$parseRaw()` result and name
metadata are `unknown`. The argument object is incorrectly treated as
another metadata object. A valid callback-backed Chat Completions tool
is also incorrectly marked non-runnable and rejected by both public
`runTools()` overloads.
A strict public-import consumer reproduced these errors before the
change. The new compile-time regressions fail on the base and pass with
the fix. They also ensure correcting the metadata cannot accidentally
make a callback-free or optional-callback tool runnable, even if the
declared metadata contains a function or the parsed argument object has
its own `name`, `arguments`, and `function` properties.
## Validation
- Complete `CI=true ./scripts/test`: 7,012 handwritten tests and 556
generated tests pass (2 existing generated skips), with all 12 generated
snapshots passing.
- Focused factory/parser suites: 46 tests pass.
- `pnpm exec tsc --noEmit`, `pnpm lint`, and `pnpm build` pass. CI's
existing type-check step includes the new compile-time assertions.
- Strict emitted-declaration consumer passes TypeScript 4.5.5
(CommonJS), 4.9 (CommonJS/ESM), and 6 (CommonJS/ESM), all with
`skipLibCheck: false`. TypeScript 4.9 also checks distributed source.
- Built CJS/ESM factory checks pass on Node 22.0.0, 24.20.0, and 26.7.0:
12 cases per runtime covering missing/sync/async callbacks, parser and
callback identity, frozen definitions, non-enumerable metadata, and
unchanged JSON serialization.
- Compared both factories' emitted CJS/ESM code with main using the
pinned compilers to normalize comments/formatting; runtime code is
unchanged.
## Adversarial self-review and scope
Reviewed the complete return-type invariant in both factories, not just
the initially failing property access. Verified callback presence is
determined by the actual callback argument rather than unrelated
parser-output properties or a possibly inaccurate metadata declaration.
Covered parse-only and optional tools in both runner modes, sync/async
callbacks, metadata without a function field, string/array/nested parser
outputs, and incompatible callback argument types. Rechecked the final
diff after validation.
This is a types-only change in handwritten code. No runtime parser,
schema conversion, request serialization, generated file, dependency,
custom-code budget, or runtime policy changes. All three paths are
absent from the pinned generated snapshot. Open-PR checks found no
overlap; openai#2517 changes the Zod convenience helper, not these custom
factories.
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.
Problem
zodFunction()currently marks every returned tool as runnable, even when its optionalfunctioncallback is omitted:This compiles on main, but both streaming and non-streaming runners reject before making a request because the tool has no associated function. The existing
standardFunction()helper already distinguishes callback-free tools.Changes
zodFunction()two overloads: a guaranteed callback produces a runnable tool; an omitted, undefined, or potentially undefined callback produces a parse-only tool.@ts-expect-errorfrom the implementation.runTools()modes, parse/stream acceptance, Zod v3/v4/Mini schemas, explicit generic calls, and promise-returning callbacks.This is a typing-only correction in two handwritten files. The function body, generated schemas, parser behavior, callback execution, and emitted runtime JavaScript are unchanged. Both files are absent from the pinned generated snapshot; no generated or vendored parser code is modified, and the open Zod schema-conversion PRs are unaffected.
Compatibility
Callback-bearing calls remain runnable. Callback-free tools remain valid for
parse()andstream(); they now fail type-checking when passed torunTools(), matching its existing runtime rejection. If a callback's type includesundefined, narrow it before creating a tool intended for execution.Validation
On Node.js 24.20.0 with pinned pnpm 11.5.1:
runTools()calls were accepted.pnpm exec tsc: passes with the fix.pnpm lintandpnpm build: passed.CI=true ./scripts/test: 7,009 handwritten and 556 generated tests passed; two existing generated tests skipped.runTools()assertions.git diff --check: passed.Adversarial self-review
Reviewed callback presence, explicit
undefined, callback unions, contextual and explicit-generic inference, async callbacks, both streaming modes, declaration compatibility, and runtime/security effects. Tightened the callback metadata and compatibility controls, then repeated review and validation. No remaining findings; no new runtime logic or dependencies.