fix(cron): allow null to clear model/fallbacks/thinking overrides on patch by fangweili · Pull Request #67429 · openclaw/openclaw

fangweili · 2026-04-15T23:23:10Z

Problem

Passing null for payload.model, payload.fallbacks, or payload.thinking in a cron.update call silently no-ops — the field cannot be cleared once set. The only workaround is manually editing ~/.openclaw/cron/jobs.json with the gateway stopped.

Fix

Treat null explicitly as "delete this key" in mergeCronPayload, mirroring the existing pattern already used for toolsAllow.

Changes

src/cron/types.ts: add string | null to model/fallbacks/thinking in CronAgentTurnPayloadPatch with JSDoc explaining null semantics
src/cron/service/jobs.ts: add null-clear branches in mergeCronPayload; coerce null → undefined in buildPayloadFromPatch
src/gateway/protocol/schema/cron.ts: allow null in CronPayloadPatchSchema for model/fallbacks/thinking (patch only, not create)
src/cron/service/jobs.apply-patch.test.ts: 5 new tests covering set, null-clear, and absent (no-op) for all three fields

Testing

All 620 cron tests pass (pnpm vitest run src/cron/)
Full pre-commit checks pass (pnpm check)
AI-assisted (OpenClaw/Boomer) — author reviewed and understands all changes

greptile-apps · 2026-04-15T23:24:53Z

Greptile Summary

Adds null-as-clear semantics for model, fallbacks, and thinking in cron.update patch payloads, mirroring the existing toolsAllow: null pattern. The change spans four files: TypeScript types, service logic, gateway schema, and tests — all consistently aligned.

Confidence Score: 5/5

Safe to merge — additive schema change, correct null-coercion, and no regressions against the existing patch contract.

All changes follow the already-established toolsAllow pattern. The null ?? undefined coercion in buildPayloadFromPatch is correct, delete next.model branches in mergeCronPayload are symmetric with the existing toolsAllow delete branch, and the TypeBox schema evolution is purely additive. No P0 or P1 findings.

No files require special attention.

_{Reviews (1): Last reviewed commit: "fix(cron): allow null to clear model/fal..." | Re-trigger Greptile}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9ed477347a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-15T23:27:01Z

+    modelNullable: true,
+    fallbacksNullable: true,
+    thinkingNullable: true,


Preserve null override fields during cron.update normalization

Allowing null in CronPayloadPatchSchema here does not actually enable clearing overrides for gateway callers, because cron.update normalizes params.patch before validation and update (normalizeCronJobPatch in src/gateway/server-methods/cron.ts), and coercePayload currently deletes payload.model, payload.thinking, and payload.fallbacks when they are null (it only preserves strings/arrays, unlike toolsAllow which uses allowNull). In practice, requests like payload: { kind: "agentTurn", model: null } are still normalized into a no-op, so the new patch semantics are unreachable through the main API path.

Useful? React with 👍 / 👎.

fangweili · 2026-04-16T00:25:33Z

Good catch, @chatgpt-codex-connector! Fixed in the latest commit.

coercePayload in src/cron/normalize.ts was stripping null for model, thinking, and fallbacks before the value reached mergeCronPayload — so the null-clear semantics were unreachable through the main API path.

Fix:

model and thinking: added explicit null guard before calling parseOptionalField (which only returns strings)
fallbacks: changed to normalizeTrimmedStringArray(next.fallbacks, { allowNull: true }) — matching the existing toolsAllow pattern

Added 3 new tests in normalize.test.ts covering null round-trip for all three fields through normalizeCronJobPatch + validateCronUpdateParams.

chatgpt-codex-connector · 2026-04-16T00:25:42Z

To use Codex here, create an environment for this repo.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d3bd5c06cf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-16T00:30:37Z

  if ("model" in next) {
-    const model = parseOptionalField(TrimmedNonEmptyStringFieldSchema, next.model);
-    if (model !== undefined) {
-      next.model = model;
+    if (next.model === null) {
+      next.model = null;
    } else {


Keep null-clearing semantics scoped to cron.update patches

coercePayload runs for both normalizeCronJobPatch and normalizeCronJobCreate, so preserving model: null here (and the analogous thinking/fallbacks branches below) changes cron.add behavior too: create requests that include nullable placeholders now reach schema validation with null still present and are rejected because CronPayloadSchema only accepts string/array values for those fields. Before this change, those nulls were dropped during normalization, so existing callers that serialize optional fields as null will regress from successful creates to INVALID_REQUEST failures.

Useful? React with 👍 / 👎.

…patch Passing null for payload.model, payload.fallbacks, or payload.thinking in a cron.update call previously silently no-oped — the field could not be cleared once set. The only workaround was manually editing jobs.json with the gateway stopped. Fix: treat null explicitly as 'delete this key' in mergeCronPayload, mirroring the existing pattern already used for toolsAllow. Changes: - src/cron/types.ts: add string|null to model/fallbacks/thinking in CronAgentTurnPayloadPatch (with JSDoc explaining null semantics) - src/cron/service/jobs.ts: add null-clear branches in mergeCronPayload; coerce null to undefined in buildPayloadFromPatch - src/gateway/protocol/schema/cron.ts: allow null in CronPayloadPatchSchema for model/fallbacks/thinking (patch only, not create) - src/cron/service/jobs.apply-patch.test.ts: add 5 tests covering set, null-clear, and absent (no-op) for all three fields All 620 cron tests pass.

fangweili · 2026-04-16T00:31:28Z

Follow-up to the previous fix: added 3 end-to-end tests in jobs.apply-patch.test.ts that wire normalizeCronJobPatch → applyJobPatch together for all three fields (model, fallbacks, thinking), confirming null survives the full normalize pipeline and results in the field being deleted from the job payload.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a00d688c97

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-16T00:35:32Z

+    if (next.model === null) {
+      next.model = null;
    } else {


Keep null-clearing logic out of cron.add normalization

coercePayload is used by both normalizeCronJobPatch and normalizeCronJobCreate, so preserving model: null here also changes create normalization. In the cron.add path (src/gateway/server-methods/cron.ts), the normalized object is validated against CronPayloadSchema, which still requires string/array values, so create requests that previously worked when clients serialized optional overrides as null now return INVALID_REQUEST; the same regression is introduced for thinking and fallbacks in the adjacent branches. Fresh evidence: this commit changed create-time coercion at this site while CronPayloadSchema remains non-nullable for those fields.

Useful? React with 👍 / 👎.

clawsweeper · 2026-04-27T04:49:51Z

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Keep this PR open because the fallbacks/thinking null-clear behavior remains useful, but the submitted branch is not merge-ready: it is conflicting against current main, lacks real behavior proof, and moves null preservation into shared create/update normalization in a way that can break existing cron.add callers.

Canonical path: Close this stale PR. The latest review rated it F, the branch still lacks merge-ready proof, and there has been no human follow-up after the durable review.

So I’m closing this here because the remaining work is already tracked in the canonical issue.

Review details

Best possible solution:

Close this stale PR. The latest review rated it F, the branch still lacks merge-ready proof, and there has been no human follow-up after the durable review.

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

Yes, from source inspection: current main drops fallbacks: null and thinking: null during patch normalization and the service never deletes those fields on null, so cron.update cannot clear them. I did not run a live gateway repro in this read-only review.

Is this the best way to solve the issue?

No, not as submitted. The narrow fix should preserve null only for update patches, not in the shared create/update normalizer, and should be rebased onto the current packages/gateway-protocol path.

Security review:

Security review cleared: No concrete security or supply-chain concern found; the diff is limited to cron/gateway protocol, service/types, and tests.

AGENTS.md: found and applied where relevant.

What I checked:

stale F-rated PR: PR was opened 2026-04-15T23:23:10Z, is older than 60 days, and the latest review rated it F.
proof blocker: real behavior proof is mock_only and proof tier is F, so this branch is not merge-ready without contributor follow-up.
no human follow-up: live comments and timeline hydrated by apply contain no non-automation activity after the ClawSweeper review.

Likely related people:

849261680: Authored the current main commit that added model null-clear semantics across the same cron normalization, service, protocol, docs, and tests surface. (role: recent related contributor; confidence: high; commits: cfeaf6897fd8; files: src/cron/normalize.ts, src/cron/service/jobs.ts, src/cron/types.ts)
steipete: Recent cron delivery-clear, failure-destination, normalization, and documentation work touched the same cron service and gateway contract area. (role: recent area contributor; confidence: medium; commits: d86b6da0120a, ee6373aa5fec, 45b5f876ddf0; files: src/cron/service/jobs.ts, src/cron/normalize.ts, src/cron/types.ts)
mbelinky: Recently landed command-job support touching cron service, normalization, types, and protocol files around the same payload patch paths. (role: recent adjacent contributor; confidence: medium; commits: b8adc11977ab; files: src/cron/service/jobs.ts, src/cron/normalize.ts, src/cron/types.ts)

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

clawsweeper · 2026-04-29T16:15:10Z

Codex review: needs changes before merge.

What this changes:

The PR adds null-as-clear semantics for cron.update payload.model, payload.fallbacks, and payload.thinking across cron patch types, gateway schema, normalization, service merge logic, and regression tests.

Required change before merge:

This is a concrete, narrow PR-branch repair: scope null preservation to cron.update patches while preserving cron.add create behavior, with clear files and validation commands.

Review details

Best possible solution:

Land a revised version that defines null-clear as a cron.update patch-only contract, preserves create-time optional-null tolerance, deletes the three stored payload overrides when patch values are null, and covers the normalize-to-apply path plus create compatibility in regression tests.

Acceptance criteria:

pnpm test src/cron/normalize.test.ts src/cron/service/jobs.apply-patch.test.ts src/cron/service.jobs.test.ts src/gateway/server-methods/cron.validation.test.ts
pnpm exec oxfmt --check --threads=1 src/cron/normalize.ts src/cron/types.ts src/cron/service/jobs.ts src/gateway/protocol/schema/cron.ts src/cron/normalize.test.ts src/cron/service/jobs.apply-patch.test.ts
pnpm check:changed in Testbox before handoff

What I checked:

Current main update path normalizes patches before validation: cron.update calls normalizeCronJobPatch before validateCronUpdateParams, so null-clear values must survive normalization to reach applyJobPatch. (src/gateway/server-methods/cron.ts:267, acae48b790fa)
Current main strips null for requested fields: coercePayload parses model and thinking through string-only optional parsing and normalizes fallbacks without allowNull; toolsAllow is the only adjacent field that preserves null. (src/cron/normalize.ts:200, acae48b790fa)
Current main merge ignores null for requested fields: mergeCronPayload sets model, fallbacks, and thinking only when the patch has string/array values; only toolsAllow has a null delete branch. (src/cron/service/jobs.ts:713, acae48b790fa)
Current protocol schema is patch-nullable only for toolsAllow: CronPayloadPatchSchema passes a nullable schema only for toolsAllow; model, fallbacks, and thinking remain non-nullable through the shared agent-turn payload schema. (src/gateway/protocol/schema/cron.ts:159, acae48b790fa)
PR review concern remains concrete: The PR diff preserves null inside coercePayload, but normalizeCronJobCreate and normalizeCronJobPatch both call normalizeCronJobInput, so null preservation must be scoped to patch normalization to avoid changing cron.add validation behavior. (src/cron/normalize.ts:646, acae48b790fa)
Existing tests cover only the old null-clear pattern: Current tests assert toolsAllow null-clear through normalization and patch application, but searches found no current-main coverage for model:null, fallbacks:null, or thinking:null clearing. (src/cron/normalize.test.ts:869, acae48b790fa)

Likely related people:

steipete: Recent commit history for the central cron service, normalization, and gateway schema paths shows repeated maintenance and refactors around this surface. (role: recent maintainer / adjacent owner; confidence: high; commits: 4bca42d9331f, df91db906fe4, b6be42230672; files: src/cron/service/jobs.ts, src/cron/normalize.ts, src/gateway/protocol/schema/cron.ts)
brunolorente: The typed cron job/patch schema commit touched all central files involved here and added the current toolsAllow patch contract that the PR mirrors. (role: introduced typed cron patch/schema behavior; confidence: medium; commits: ca76e2fedc8e; files: src/cron/normalize.ts, src/cron/service/jobs.ts, src/cron/types.ts)
andyk-ms: The per-job tools allow-list feature introduced the explicit null-as-clear normalization pattern used as the precedent for this PR. (role: adjacent owner for toolsAllow clear semantics; confidence: medium; commits: 4d8c07b97c0b; files: src/cron/normalize.ts, src/cron/types.ts)
yinghaosang: The payload.fallbacks feature added the per-job fallback fields in cron types and gateway schema, making it relevant to the fallbacks clearing part of this change. (role: introduced payload.fallbacks contract; confidence: medium; commits: f902697bd5c4; files: src/cron/types.ts, src/gateway/protocol/schema/cron.ts)

Remaining risk / open question:

The PR branch should be revised because preserving null in the shared coercePayload path can turn previously accepted cron.add payload optional-null placeholders into validation failures.
Gateway protocol schema changes need the final schema, runtime validators, generated/client expectations, docs, and tests kept aligned.
No tests were run in this review because the requested pass was read-only; the revised branch still needs targeted cron tests and the changed gate.

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

clawsweeper · 2026-05-20T00:18:55Z

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?

The egg game starts only after the PR passes the real-behavior proof check.
Before that, no creature or rarity is rolled. The treat waits for real proof.
This is still just collectible flavor: proof affects review readiness, not creature quality.

openclaw-barnacle · 2026-06-06T04:47:48Z

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

clawsweeper · 2026-06-15T00:10:13Z

ClawSweeper applied the proposed close for this PR.

Action: closed this PR.
Close reason: duplicate or superseded.
Evidence: durable ClawSweeper review.

fangweili requested a review from a team as a code owner April 15, 2026 23:23

openclaw-barnacle Bot added app: web-ui App: web-ui gateway Gateway runtime size: S labels Apr 15, 2026

chatgpt-codex-connector Bot reviewed Apr 15, 2026

View reviewed changes

fangweili force-pushed the fix/cron-model-null-clear branch from 9ed4773 to d3bd5c0 Compare April 16, 2026 00:25

chatgpt-codex-connector Bot reviewed Apr 16, 2026

View reviewed changes

fangweili force-pushed the fix/cron-model-null-clear branch from d3bd5c0 to a00d688 Compare April 16, 2026 00:31

chatgpt-codex-connector Bot reviewed Apr 16, 2026

View reviewed changes

openclaw-barnacle Bot added the triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. label May 19, 2026

clawsweeper Bot mentioned this pull request Jun 4, 2026

[Feature]: Expose --fallbacks on openclaw cron create/edit #90302

Closed

openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jun 6, 2026

openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label Jun 7, 2026

clawsweeper Bot removed the rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. label Jun 7, 2026

clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 7, 2026

ly-wang19 mentioned this pull request Jun 9, 2026

fix(cron): add cron edit --clear-model to clear a job's model override #91625

Merged

2 tasks

clawsweeper Bot closed this Jun 15, 2026

takamasa-aiso mentioned this pull request Jun 24, 2026

Bug: Cron Control UI cannot clear saved agentTurn model/thinking overrides by blanking fields #96287

Closed

clawsweeper Bot mentioned this pull request Jun 24, 2026

fix(cron): clear agentTurn thinking override by blanking the field #96293

Merged

Uh oh!

Uh oh!

Conversation

fangweili commented Apr 15, 2026

Problem

Fix

Changes

Testing

Uh oh!

greptile-apps Bot commented Apr 15, 2026

Greptile Summary

Confidence Score: 5/5

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

fangweili commented Apr 16, 2026

Uh oh!

chatgpt-codex-connector Bot commented Apr 16, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

fangweili commented Apr 16, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

clawsweeper Bot commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clawsweeper Bot commented Apr 29, 2026

Uh oh!

clawsweeper Bot commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openclaw-barnacle Bot commented Jun 6, 2026

Uh oh!

clawsweeper Bot commented Jun 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

clawsweeper Bot commented Apr 27, 2026 •

edited

Loading

clawsweeper Bot commented May 20, 2026 •

edited

Loading