Skip to content

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

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

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: needs maintainer review before merge. Reviewed June 1, 2026, 1:38 AM ET / 05:38 UTC.

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

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

Reproducibility: yes. From source, current main returns raw nullable anyOf/const schemas through the Codex dynamic-tool projection path, and the plain JSON-schema union coercer can coerce null through an earlier non-null branch; I did not run tests in this read-only review.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

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

Rank-up moves:

  • Let current-head CI and mergeability settle against the latest main before landing.
  • [P2] Maintainer should explicitly accept the null-clear schema projection tradeoff before merge.

Risk before merge

  • [P1] Provider-facing schemas now drop explicit null branches and rely on descriptions plus raw validation to preserve clear sentinels, so maintainers should accept that compatibility tradeoff before merge.
  • [P1] The PR body reports focused tests, snapshots, lint, diff-check, scrub, and autoreview, but no live provider turn or full changed gate; GitHub CI should settle the moving-base integration state.

Maintainer options:

  1. Accept the projection tradeoff (recommended)
    Maintainers can accept the schema-contract change because the raw validation path still preserves null clears and focused tests cover the affected cron and dynamic-tool shapes.
  2. Ask for live provider proof
    If maintainers want extra confidence, request one live provider turn that registers the cron dynamic tool and exercises a nullable clear without provider schema rejection.
  3. Pause for API direction
    If provider-facing schemas must continue advertising null explicitly, pause this PR and choose a different compatibility contract before changing the shared projection helper.

Next step before merge

  • No automated repair is needed; the remaining action is maintainer review of the compatibility tradeoff plus normal CI/mergeability gating.

Security
Cleared: The diff changes TypeScript schema projection/validation tests and prompt snapshots; it does not add dependencies, lockfile changes, scripts, permissions, or executable supply-chain surface.

Review details

Best possible solution:

Land this shape if maintainers accept the provider-schema tradeoff: keep raw runtime validation nullable, project only the provider-facing schema, and let CI verify the current merge result.

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

Yes. From source, current main returns raw nullable anyOf/const schemas through the Codex dynamic-tool projection path, and the plain JSON-schema union coercer can coerce null through an earlier non-null branch; I did not run tests in this read-only review.

Is this the best way to solve the issue?

Yes. Centralizing the provider projection in projectRuntimeToolInputSchema while keeping raw validation separate is the narrowest maintainable fix; duplicating cron-specific or Codex-specific schema rewrites would be more drift-prone.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 6173a4babb99.

Label changes

Label changes:

  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The external-contributor real-proof gate does not apply to this maintainer/MEMBER PR; the body records focused tests, snapshot, lint, diff-check, scrub, and autoreview proof.
  • remove rating: 🌊 off-meta tidepool: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.

Label justifications:

  • P2: This is a normal-priority provider/agent tool-schema compatibility fix with focused blast radius and no emergency runtime outage evidence.
  • merge-risk: 🚨 compatibility: The PR intentionally changes the provider-facing tool schema contract by removing explicit null and const branches before dynamic tools reach providers.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The external-contributor real-proof gate does not apply to this maintainer/MEMBER PR; the body records focused tests, snapshot, lint, diff-check, scrub, and autoreview proof.
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:

  • Current main sends raw runtime schemas through the projection helper: On current main, projectRuntimeToolInputSchema serializes and validates the schema but returns projection.schema unchanged, so nullable anyOf/const shapes can reach Codex dynamic tool inputSchema unchanged. (src/agents/tool-schema-projection.ts:184, 6173a4babb99)
  • PR adds provider-side schema normalization at the shared projection point: The PR normalizes anyOf/oneOf compositions after raw diagnostics, dropping null variants for provider schemas and folding string const unions to string enums before returning the projected schema. (src/agents/tool-schema-projection.ts:167, 8aeaadf963a3)
  • Raw validation remains separate from provider projection: The PR adds a null pre-check in plain JSON-schema union coercion so explicit null values accepted by a union are not coerced through an earlier number/string branch before validation. (packages/llm-core/src/validation.ts:206, 8aeaadf963a3)
  • Cron schema is a central affected runtime schema: Cron declares nullable string, array, delivery, and literal-union TypeBox schemas that explain null clear sentinels in descriptions, making it a representative affected tool surface for this projection change. (src/agents/tools/cron-tool.ts:66, 6173a4babb99)
  • Codex dynamic tool contract inspected: Sibling Codex source defines DynamicToolSpec.input_schema as arbitrary JsonValue and validates each dynamic tool with codex_tools::parse_tool_input_schema before accepting thread/start dynamic tools. (codex-rs/app-server/src/request_processors/thread_processor.rs:316)
  • Codex parser accepts nullable fields but rejects singleton null schemas: Sibling Codex parser supports JSON Schema null as a primitive type but rejects a singleton top-level null schema, matching the PR's choice to preserve object roots while removing provider-facing null branches where needed. (codex-rs/tools/src/json_schema.rs:11)

Likely related people:

  • Vincent Koc: Current-main blame and log attribute the shared schema projection, validation, and recent agent schema refactor lines to Vincent Koc, and this PR's commits continue the same provider-schema surface. (role: recent area contributor; confidence: high; commits: aab1e727c617, b071eb190dd2, bf275a59f0b8; files: src/agents/tool-schema-projection.ts, packages/llm-core/src/validation.ts, src/agents/tools/cron-tool.ts)
  • Kip: Recent cron history and blame show Kip touched nearby cron schema/runtime lines, so they are a useful secondary routing candidate for cron-specific nullable clear behavior. (role: recent cron adjacent contributor; confidence: medium; commits: c213827aa50b; files: src/agents/tools/cron-tool.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 status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 1, 2026
@vincentkoc vincentkoc force-pushed the cron-schema-main-repro-20260601 branch from c6aaf80 to a23ee0b Compare June 1, 2026 04:28
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 1, 2026
@vincentkoc vincentkoc force-pushed the cron-schema-main-repro-20260601 branch from a23ee0b to 4caf7f9 Compare June 1, 2026 05:01
@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
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: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: L status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@vincentkoc