Skip to content

fix(agents): project nullable tool schemas for providers#88880

Closed
vincentkoc wants to merge 6 commits into
mainfrom
cron-schema-main-repro-20260601
Closed

fix(agents): project nullable tool schemas for providers#88880
vincentkoc wants to merge 6 commits into
mainfrom
cron-schema-main-repro-20260601

Conversation

@vincentkoc

@vincentkoc vincentkoc commented Jun 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Summary

  • project nullable and string-literal tool schemas into provider-compatible shapes before sending runtime tool schemas to providers
  • keep unsupported dynamic-schema diagnostics on the raw serialized schema so projection cannot hide a blocker
  • preserve explicit null clear sentinels during raw argument validation before provider projection
  • preserve non-null union branches when dropping nullable variants, instead of narrowing mixed unions to string
  • refresh only the prompt snapshots affected by the provider-facing dynamic tool schema

Verification

Reviewed/proved local head: 8aeaadf963a
Reviewed/proved base: db4990d2605
Current note: origin/main advanced again after the final review; GitHub CI/mergeability should arbitrate the pushed branch against the newest base.

  • 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 / 118 tests
  • node --import tsx scripts/generate-prompt-snapshots.ts --check - passed, Prompt snapshots are current (7 files).
  • 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 src/agents/tools/cron-tool.test.ts - passed
  • node scripts/run-oxlint.mjs --tsconfig config/tsconfig/oxlint.json 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 src/agents/tools/cron-tool.test.ts - passed
  • git diff --check origin/main...HEAD - passed on the reviewed base
  • private reported-name scrub across the spec, changed TS files, and prompt snapshots - clean
  • .agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main --prompt 'Review the pruned provider/runtime tool schema projection hardening branch after rebasing onto latest main. Focus only on actionable regressions in provider-facing schema projection, raw validation preservation, non-null union preservation, and prompt snapshot correctness. Do not inspect external repositories unless essential.' - clean, no accepted/actionable findings, overall: patch is correct (0.76)

Real behavior proof

Behavior addressed: Provider-facing runtime tool schema projection no longer leaks nullable OpenAPI-incompatible shapes for covered runtime schemas, while raw validation still accepts explicit null clear sentinels and mixed non-null unions remain represented as unions.

Real environment tested: Local OpenClaw Codex/gwt worktree on macOS with shared repo node_modules, using the repo Vitest wrapper plus snapshot, format, lint, diff, scrub, and autoreview gates.

Exact steps or command run after this patch: The focused Vitest, prompt snapshot, oxfmt, oxlint, diff-check, private-name scrub, and autoreview commands listed above were run after the final rebase.

Evidence after fix: Focused tests passed 3 shards / 118 tests; prompt snapshots were current; oxfmt, oxlint, and diff-check passed; autoreview reported no accepted/actionable findings.

Observed result after fix: Nullable provider schema branches are projected to provider-compatible schemas, unsupported dynamic keywords are still reported from raw schemas, explicit null clear values validate correctly, and mixed non-null unions are preserved instead of narrowed.

What was not tested: A live provider turn or full pnpm check:changed was not run for this narrow PR; GitHub CI is expected to cover the pushed branch against the latest moving main.

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: M maintainer Maintainer-authored PR labels Jun 1, 2026
@vincentkoc
vincentkoc marked this pull request as ready for review June 1, 2026 02:46
@clawsweeper

clawsweeper Bot commented Jun 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: found issues before merge. Reviewed June 20, 2026, 4:15 AM ET / 08:15 UTC.

Summary
The PR projects nullable and string-literal runtime tool schemas into provider-compatible shapes, preserves raw validation semantics for null clears, and refreshes affected Codex prompt snapshots.

PR surface: Source +93, Tests -126. Total -33 across 12 files.

Reproducibility: yes. at source level: current main has nullable cron clear schemas and the active projectRuntimeToolInputSchema helper returns serialized schemas unchanged, while related provider-normalizer evidence shows strict Gemini-compatible providers reject those shapes. I did not run a live provider turn in this read-only review.

Review metrics: 2 noteworthy metrics.

  • Shared Projection Boundary: 1 shared runtime/Codex projection path changed. The changed schema shape is visible to multiple provider and Codex consumers, so compatibility needs maintainer acceptance before merge.
  • Overlapping Fix Paths: 2 open related PRs plus 1 open issue. The same provider-schema symptom is also being handled by narrower related work, so maintainers should pick one landing direction.

Stored data model
Persistent data-model change detected: serialized state: src/agents/tools/cron-tool.schema.test.ts, 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 and port the retained change to the active current-main schema helper or provider normalizer.
  • [P2] Get maintainer direction on broad shared projection versus the narrower provider/model-specific fix path.
  • Rerun focused schema, validation, prompt snapshot, lint, and conflict checks on the rebased head.

Risk before merge

  • [P1] The branch is currently conflicting and changes an older projection implementation, while current main routes projectRuntimeToolInputSchema through src/agents/tool-schema-json-projection.ts.
  • [P1] The PR drops explicit nullable/literal variants from a shared runtime/Codex-facing schema projection path, which changes model-visible tool contracts even if raw validation still accepts null clear sentinels.
  • [P1] The concrete Gemini/OpenAI-compatible cron rejection overlaps narrower open work, especially fix(agents): clean Gemini tool schemas by model id #91559, so maintainers need to choose broad shared projection versus provider/model-specific normalization.

Maintainer options:

  1. Rebase To Current Boundary (recommended)
    Port any retained nullable/literal projection to src/agents/tool-schema-json-projection.ts or the provider normalizer and rerun focused schema, validation, snapshot, and conflict checks.
  2. Accept Broad Schema Contract Change
    Maintainers can explicitly accept that shared provider/Codex-facing schemas omit nullable variants while runtime validation keeps null clear sentinels.
  3. Defer To Narrower Provider Fix
    If the broad projection is not desired, keep the narrower provider/model-specific PR as the landing path for the Gemini issue and close or replace this stale branch later.

Next step before merge

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

Security
Cleared: The diff changes TypeScript schema projection/validation code, tests, and snapshots without dependency, lockfile, workflow, permission, credential, or package-publishing changes.

Review findings

  • [P1] Port projection to the current schema boundary — src/agents/tool-schema-projection.ts:281
Review details

Best possible solution:

Rebase first, then either port the accepted normalization into the active current-main schema boundary with explicit maintainer approval for the broad contract change, or defer the concrete Gemini regression to #91559.

Do we have a high-confidence way to reproduce the issue?

Yes, at source level: current main has nullable cron clear schemas and the active projectRuntimeToolInputSchema helper returns serialized schemas unchanged, while related provider-normalizer evidence shows strict Gemini-compatible providers reject those shapes. I did not run a live provider turn in this read-only review.

Is this the best way to solve the issue?

No, not as submitted: the branch targets a stale helper and applies cleanup through a broad runtime/Codex-facing projection path. The safer path is a rebase plus maintainer choice between broad shared projection and narrower provider/model-specific normalization.

Full review comments:

  • [P1] Port projection to the current schema boundary — src/agents/tool-schema-projection.ts:281
    Current main now re-exports projectRuntimeToolInputSchema from src/agents/tool-schema-json-projection.ts, but this branch returns normalized schemas from the older implementation and GitHub reports the PR as conflicting. Rebase and move any retained normalization to the active helper or provider normalizer before merge so the fix lands on the runtime path being called.
    Confidence: 0.92

Overall correctness: patch is incorrect
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🌊 off-meta tidepool and patch quality is 🧂 unranked krab.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🧂 unranked krab, so this older rating label is no longer current.

Label justifications:

  • P2: This is normal-priority agent/provider compatibility work with bounded blast radius and no data-loss, security, or crash-loop evidence.
  • merge-risk: 🚨 compatibility: The PR changes model/provider-visible tool schema contracts by dropping nullable/literal variants from a shared runtime/Codex-facing projection path.
  • 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 PR; the body provides focused local validation but no live provider turn.
Evidence reviewed

PR surface:

Source +93, Tests -126. Total -33 across 12 files.

View PR surface stats
Area Files Added Removed Net
Source 2 100 7 +93
Tests 10 351 477 -126
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 12 451 484 -33

What I checked:

  • Repository policy applied: Root and scoped AGENTS.md files were read; the relevant policy treats provider/plugin schema behavior and Codex-facing changes as compatibility-sensitive and requires direct Codex source inspection. (AGENTS.md:14, a1271830948b)
  • Live PR state: GitHub reports this PR open, non-draft, protected by the maintainer label, head 8aeaadf, and mergeable=CONFLICTING / mergeStateStatus=DIRTY. (8aeaadf963a3)
  • Current main projection boundary: Current main makes src/agents/tool-schema-projection.ts a wrapper that re-exports projectRuntimeToolInputSchema from src/agents/tool-schema-json-projection.ts. (src/agents/tool-schema-projection.ts:1, a1271830948b)
  • Active helper still returns raw schema: The active current-main helper serializes and checks dynamic keywords, then returns projection.schema unchanged, so the PR-head normalization is not present in the active file. (src/agents/tool-schema-json-projection.ts:115, a1271830948b)
  • PR implementation target: The PR head returns normalizeProviderToolInputSchema(serialized.schema) from the older projectRuntimeToolInputSchema implementation, which must be ported or intentionally rejected against current main. (src/agents/tool-schema-projection.ts:281, 8aeaadf963a3)
  • Provider normalizer sibling path: Current main already centralizes provider/model schema cleanup in normalizeToolParameterSchema, including provider-specific cleaning before model dispatch. (src/agents/agent-tools-parameter-schema.ts:750, a1271830948b)

Likely related people:

  • vincentkoc: Current-main blame and file history tie the active projection, provider normalizer, validation helper, and this protected PR to Vincent Koc's recent work. (role: recent area contributor; confidence: high; commits: 4f278ef71c6f, 8aeaadf963a3, 844f405ac1be; files: src/agents/tool-schema-projection.ts, src/agents/tool-schema-json-projection.ts, src/agents/agent-tools-parameter-schema.ts)
  • Bruno Lorente: History search found the earlier cron-tool typed-property schema work that made cron job/patch schemas model-facing and relevant to provider schema compatibility. (role: introduced related cron schema behavior; confidence: medium; commits: ca76e2fedc8e; files: src/agents/tools/cron-tool.ts, src/agents/tools/cron-tool.schema.test.ts)
  • Pick-cat: The newer open PR fix(agents): clean Gemini tool schemas by model id #91559 handles the concrete Gemini/OpenAI-compatible schema rejection through the provider normalizer path. (role: adjacent candidate-fix author; confidence: medium; commits: 8c05154bbe9e; files: src/agents/agent-tools-parameter-schema.ts, src/agents/agent-tools.schema.test.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.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. labels Jun 1, 2026
@vincentkoc
vincentkoc force-pushed the cron-schema-main-repro-20260601 branch from 15c66cf to c54a143 Compare June 1, 2026 03:28
@vincentkoc vincentkoc mentioned this pull request Jun 1, 2026
Closed
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jun 1, 2026
@vincentkoc
vincentkoc force-pushed the cron-schema-main-repro-20260601 branch from c54a143 to ef6bc22 Compare June 1, 2026 03:47
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jun 1, 2026
@clawsweeper clawsweeper Bot added rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jun 1, 2026
@vincentkoc
vincentkoc force-pushed the cron-schema-main-repro-20260601 branch from 4caf7f9 to 8aeaadf Compare June 1, 2026 05:30
@vincentkoc vincentkoc self-assigned this Jun 1, 2026
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. labels Jun 1, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 14, 2026
Closed
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jun 15, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 19, 2026
Closed
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jun 19, 2026
This was referenced Jun 19, 2026
Closed
Merged
Closed
@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 narrower fixes now on main.

This branch combined two real concerns: runtime validation must preserve explicit null clears, and Gemini-backed provider payloads need a compatible projected schema. Runtime null-union coercion was fixed narrowly in #97212. Provider normalization now belongs to packages/ai/src/providers/agent-tools-parameter-schema.ts (moved into @openclaw/ai by #99059), with direct Gemini/model-profile and cron projection tests; #91559 covers the Gemini-model routing case. Replaying this older six-commit projection layer would duplicate that canonical path and refresh obsolete snapshots.

Thanks for the investigation and tests, @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

Reviewers

No reviews

Assignees

@vincentkoc vincentkoc

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: L status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@vincentkoc @steipete