fix(agents): project cron tool schemas for providers#88878
Conversation
|
Codex review: found issues before merge. Reviewed June 29, 2026, 6:03 PM ET / 22:03 UTC. Summary PR surface: Source +83, Tests +109. Total +192 across 11 files. Reproducibility: yes. source-level reproduction is high-confidence: the PR diff can hide raw dynamic-schema diagnostics, narrow mixed unions, and change ordered union coercion, while current main shows the safer active boundaries. I did not run a live provider request in this read-only review. Review metrics: 2 noteworthy metrics.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Rebase first, then either port only the retained broad projection behavior to the active current-main boundary with raw diagnostics and union semantics preserved, or leave the concrete Gemini fix to the merged provider-normalizer path and close/replace this draft by maintainer choice. Do we have a high-confidence way to reproduce the issue? Yes, source-level reproduction is high-confidence: the PR diff can hide raw dynamic-schema diagnostics, narrow mixed unions, and change ordered union coercion, while current main shows the safer active boundaries. I did not run a live provider request in this read-only review. Is this the best way to solve the issue? No as submitted. Provider schema compatibility is real, but this draft targets a stale boundary and changes shared contracts too broadly; the safer path is the current provider-normalizer fix for Gemini plus a separately rebased broad projection decision if maintainers still want it. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 843ad143647e. Label changesLabel justifications:
Evidence reviewedPR surface: Source +83, Tests +109. Total +192 across 11 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (1 earlier review cycle)
|
|
Closing as superseded by the current provider-schema owner and tests. The useful intent here was to preserve raw nullable cron semantics while projecting provider-compatible schemas. Current Thanks, @vincentkoc. |
Summary
nullclear sentinels still reach cron runtime normalizationUnblocks current CI failures seen on #87856 after
d86b6da0120made cron clear fields nullable in the raw tool schema.Verification
node scripts/run-vitest.mjs packages/llm-core/src/validation.test.ts src/agents/tool-schema-projection.test.ts src/agents/tools/cron-tool.schema.test.ts src/agents/tools/cron-tool.test.ts --reporter=dot: passed, 3 shards / 114 testsnode --import tsx scripts/generate-prompt-snapshots.ts --check: passed, 7 files currentnode_modules/.bin/oxfmt --check --threads=1 packages/llm-core/src/validation.test.ts packages/llm-core/src/validation.ts src/agents/tool-schema-projection.test.ts src/agents/tool-schema-projection.ts src/agents/tools/cron-tool.schema.test.ts test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/codex-dynamic-tools.discord-group.json test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/codex-dynamic-tools.heartbeat-turn.json test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/codex-dynamic-tools.telegram-direct.json test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/discord-group-codex-message-tool.md test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/telegram-direct-codex-message-tool.md test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/telegram-heartbeat-codex-tool.md: passednode scripts/run-oxlint.mjs packages/llm-core/src/validation.test.ts packages/llm-core/src/validation.ts src/agents/tool-schema-projection.test.ts src/agents/tool-schema-projection.ts src/agents/tools/cron-tool.schema.test.ts: passedgit diff --check origin/main...HEAD: passedpnpm check:changed: attempted withnode scripts/crabbox-wrapper.mjs run --provider aws --label cron-tool-provider-schema-check-changed --shell -- "pnpm check:changed", but coordinator returnedactive lease limit exceeded: 9/8before allocation; no lease was created.Real behavior proof
Behavior addressed: provider-facing tool schemas should avoid OpenAPI 3.0-incompatible nullable/composition shapes, while runtime validation still accepts explicit
nullclear sentinels.Real environment tested: linked OpenClaw gwt worktree on current
origin/main.Exact steps or command run after this patch: targeted Vitest files for llm validation, tool-schema projection, cron schema, and cron runtime behavior; prompt snapshot check; oxfmt; oxlint; diff-check; attempted AWS Crabbox changed gate.
Evidence after fix: cron schema projection tests now see plain
string/arrayprovider fields, raw validation preservesnullclear sentinels, and prompt snapshots are current.Observed result after fix: provider schemas no longer expose
anyOf/oneOf/type: null/constfor these tool fields, but cron updates can still carrynullthrough raw validation into runtime normalization.What was not tested: remote
pnpm check:changed, blocked by AWS Crabbox lease capacity.