Skip to content

fix(agents): project cron tool schemas for providers#88878

Closed
vincentkoc wants to merge 1 commit into
mainfrom
fix-cron-tool-provider-schema-20260601
Closed

fix(agents): project cron tool schemas for providers#88878
vincentkoc wants to merge 1 commit into
mainfrom
fix-cron-tool-provider-schema-20260601

Conversation

@vincentkoc

@vincentkoc vincentkoc commented Jun 1, 2026

Copy link
Copy Markdown
Member

Summary

  • project runtime tool schemas into provider-facing OpenAPI-friendly shapes by dropping nullable composition wrappers and converting string literal unions to enums
  • keep raw TypeBox validation intact so null clear sentinels still reach cron runtime normalization
  • refresh Codex happy-path prompt snapshots that drifted on current main

Unblocks current CI failures seen on #87856 after d86b6da0120 made 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 tests
  • node --import tsx scripts/generate-prompt-snapshots.ts --check: passed, 7 files current
  • node_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: passed
  • node 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: passed
  • git diff --check origin/main...HEAD: passed
  • AWS Crabbox pnpm check:changed: attempted with node scripts/crabbox-wrapper.mjs run --provider aws --label cron-tool-provider-schema-check-changed --shell -- "pnpm check:changed", but coordinator returned active lease limit exceeded: 9/8 before 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 null clear 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/array provider fields, raw validation preserves null clear sentinels, and prompt snapshots are current.

Observed result after fix: provider schemas no longer expose anyOf/oneOf/type: null/const for these tool fields, but cron updates can still carry null through raw validation into runtime normalization.

What was not tested: remote pnpm check:changed, blocked by AWS Crabbox lease capacity.

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: M maintainer Maintainer-authored PR labels Jun 1, 2026
@clawsweeper

clawsweeper Bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Codex review: found issues before merge. Reviewed June 29, 2026, 6:03 PM ET / 22:03 UTC.

Summary
The PR projects cron/runtime tool schemas into provider-facing OpenAPI-friendly shapes, preserves raw nullable runtime validation, and refreshes affected Codex prompt snapshots.

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.

  • Shared Schema Behaviors: 2 changed: projection and union coercion. The diff touches both model-visible schema projection and llm-core argument validation, so the blast radius is broader than a cron-only test refresh.
  • Overlapping Fix Paths: 1 merged related PR and 1 open broader PR. Maintainers need to decide whether this draft still carries unique broad projection work after the focused Gemini fix merged and the broader successor remains open.

Stored data model
Persistent data-model change detected: vector/embedding metadata: src/agents/tool-schema-projection.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🌊 off-meta tidepool
Patch quality: 🧂 unranked krab
Result: blocked by patch quality or review findings.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Rebase or port the retained behavior to the active current-main schema boundary.
  • [P2] Fix raw-diagnostics ordering, mixed-union preservation, and llm-core union coercion semantics.
  • Get maintainer direction on broad shared projection versus the focused provider-normalizer path.

Risk before merge

  • [P1] The branch is draft, conflicting, and targets an older projection implementation while current main routes the active helper through src/agents/tool-schema-json-projection.ts.
  • [P1] The diff changes shared model/provider/Codex-facing schema projection and llm-core union coercion, so it can alter tool contracts and argument validation beyond cron.
  • [P1] The concrete jjcc/Gemini cron rejection is already fixed on current main by fix(agents): clean Gemini tool schemas by model id #91559, while fix(agents): project nullable tool schemas for providers #88880 remains the broader open schema-projection discussion.

Maintainer options:

  1. Port Retained Behavior To Current Boundary (recommended)
    Move any accepted nullable/literal projection onto the active current-main helper or provider normalizer, keep raw diagnostics before provider cleanup, and preserve ordered union coercion semantics.
  2. Keep The Focused Gemini Fix As Canonical
    If maintainers only need the jjcc/Gemini regression fixed, rely on the merged provider-normalizer fix and close or replace this broader draft after preserving useful context.
  3. Accept A Broad Schema Contract Change
    Maintainers may explicitly accept that shared provider/Codex-facing schemas drop nullable and literal composition details while runtime validation keeps the raw null-clear contract.

Next step before merge

  • [P2] Manual review is needed because this protected maintainer draft is conflicting and requires a scope decision before any automated repair lane would know which schema boundary to preserve.

Security
Cleared: No concrete security or supply-chain issue was found; the diff changes TypeScript schema projection/validation logic, tests, and prompt snapshots only.

Review findings

  • [P2] Run diagnostics before provider projection — src/agents/tool-schema-projection.ts:135
  • [P2] Preserve mixed non-null union alternatives — src/agents/tool-schema-projection.ts:188-193
  • [P2] Keep non-null union coercion ordered — packages/llm-core/src/validation.ts:207-212
Review details

Best 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:

  • [P2] Run diagnostics before provider projection — src/agents/tool-schema-projection.ts:135
    projectRuntimeToolInputSchema now returns the provider-normalized tree before dynamic-schema diagnostics run. A raw branch containing $dynamicRef or $dynamicAnchor can be collapsed or replaced before quarantine sees it, so keep diagnostics on the serialized raw schema and only project for provider output afterward.
    Confidence: 0.9
  • [P2] Preserve mixed non-null union alternatives — src/agents/tool-schema-projection.ts:188-193
    The normalization falls back to the first string variant whenever a composition contains one, so string | number | null becomes only string. That misrepresents supported number inputs to providers and SDK consumers; dropping null branches must not discard other non-null alternatives.
    Confidence: 0.86
  • [P2] Keep non-null union coercion ordered — packages/llm-core/src/validation.ts:207-212
    The exact-match pre-pass returns the original value for any matching union branch before ordered coercion. For anyOf [{type:"number"}, {type:"string"}], a numeric string now stays a string instead of following the existing first-branch numeric coercion; scope the shortcut to explicit null preservation.
    Confidence: 0.87

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 843ad143647e.

Label changes

Label justifications:

  • P2: This is normal-priority agent/provider compatibility work with concrete blockers and a bounded provider-schema blast radius.
  • merge-risk: 🚨 compatibility: The PR changes shared provider/Codex-visible tool schema projection and llm-core validation behavior used across provider and runtime paths.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🌊 off-meta tidepool and patch quality is 🧂 unranked krab.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The external-contributor proof gate is not applicable to this MEMBER-authored maintainer-labeled PR; the PR body provides targeted local validation but no live provider turn.
Evidence reviewed

PR surface:

Source +83, Tests +109. Total +192 across 11 files.

View PR surface stats
Area Files Added Removed Net
Source 2 84 1 +83
Tests 9 178 69 +109
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 11 262 70 +192

What I checked:

  • Repository policy applied: Root AGENTS.md was read fully, along with scoped src/agents, src/agents/tools, and test guides; provider/tool schema behavior is compatibility-sensitive and Codex-facing changes require direct sibling Codex source inspection. (AGENTS.md:14, 843ad143647e)
  • Live PR state: GitHub reports this PR open, draft, maintainer-labeled, head daa3581, and mergeable=CONFLICTING / mergeStateStatus=DIRTY. (daa3581013fe)
  • PR projects before raw diagnostics: The diff changes serialization to return normalizeProviderToolInputSchema(parsed), which means raw dynamic-schema diagnostics can inspect a provider-normalized tree instead of the original schema. (src/agents/tool-schema-projection.ts:135, daa3581013fe)
  • Current main projection boundary: Current main makes src/agents/tool-schema-projection.ts a wrapper and keeps raw JSON projection and dynamic-keyword diagnostics in src/agents/tool-schema-json-projection.ts. (src/agents/tool-schema-projection.ts:1, 843ad143647e)
  • Current main raw projection behavior: The active helper serializes, validates object shape, reports dynamic keywords, and returns projection.schema unchanged; broad provider cleanup is not in this helper on current main. (src/agents/tool-schema-json-projection.ts:115, 843ad143647e)
  • Provider normalizer already owns the focused Gemini fix: Current main selects Gemini cleanup from provider id, Gemini model id, or toolSchemaProfile, and applies cleanSchemaForGemini in normalizeToolParameterSchemaUncached. (src/agents/agent-tools-parameter-schema.ts:758, 843ad143647e)

Likely related people:

  • vincentkoc: Recent current-main history touches the tool schema helper exports, and this maintainer-authored draft plus the overlapping broader schema PR are in the same provider/runtime schema area. (role: recent area contributor; confidence: high; commits: e7e686db2d00, 361869e43422, daa3581013fe; files: src/agents/tool-schema-projection.ts, src/agents/tool-schema-json-projection.ts, packages/llm-core/src/validation.ts)
  • Pick-cat: Authored the merged provider-normalizer fix that handles Gemini model ids behind OpenAI-compatible providers for the concrete cron schema rejection. (role: adjacent focused-fix author; confidence: medium; commits: 1f89d6d7f762; files: src/agents/agent-tools-parameter-schema.ts, src/agents/agent-tools.schema.test.ts, src/agents/tools/cron-tool.schema.test.ts)
  • steipete: History shows substantial work on the provider schema normalizer and Gemini schema cleaner used by this path. (role: recent adjacent contributor; confidence: high; commits: 60e0d2a7b910, fe1fd055d557, 33aea44fe5b0; files: src/agents/agent-tools-parameter-schema.ts, src/agents/schema/clean-for-gemini.ts)
  • openperf: Earlier merged cron/Gemini compatibility work removed OpenAPI 3.0-incompatible cron schema keywords in the same provider compatibility surface. (role: prior provider-schema contributor; confidence: medium; commits: 8c1ca1f24517; files: src/agents/schema/clean-for-gemini.ts, src/agents/tools/cron-tool.ts, src/agents/tools/cron-tool.schema.test.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (1 earlier review cycle)
  • reviewed 2026-06-21T07:43:23.741Z sha daa3581 :: found issues before merge. :: [P2] Run diagnostics before provider projection | [P2] Preserve mixed non-null union alternatives | [P2] Keep non-null union coercion ordered

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jun 1, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jun 20, 2026
@steipete

steipete commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

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 main now does that at the canonical provider boundary: normalizeToolParameterSchema lives in packages/ai/src/providers/agent-tools-parameter-schema.ts, Gemini cleanup is selected by provider/model/profile, and src/agents/tools/cron-tool.schema.test.ts checks both raw null clears and Gemini-facing projections. #91559 and the later #99059 ownership move cover the provider path without adding another normalization stack to tool-schema-projection.ts; #97212 separately fixed runtime null-union coercion.

Thanks, @vincentkoc.

@steipete steipete closed this Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling maintainer Maintainer-authored PR merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: M status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants