fix(cron): strip internal whitespace from model IDs in cron job normalization by Angfr95 · Pull Request #66543 · openclaw/openclaw

Angfr95 · 2026-04-14T12:41:08Z

Summary

Problem: Cron tool preserves internal whitespace in model IDs (e.g. custom-1/ minimax/ minimax-m2.5-free), causing OpenRouter to reject them with 400 ... is not a valid model ID (format).
Why it matters: All cron jobs using affected model IDs fail silently, and users cannot fix them via update/delete due to the same corruption path.
What changed: coercePayload() and copyTopLevelAgentTurnFields() in src/cron/normalize.ts now strip all internal whitespace from model and fallbacks fields via .replace(/\s+/g, '').
What did NOT change (scope boundary): No changes to store serialization, schedule normalization, delivery logic, or any other payload fields.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Closes [Bug]: Cron Tool Corrupts Model IDs #66203
This PR fixes a bug or regression

Root Cause (if applicable)

Root cause: TrimmedNonEmptyStringFieldSchema only .trim()s leading/trailing whitespace. LLMs frequently insert spaces around / and - separators when generating tool call parameters. The normalization layer had no guard against internal whitespace in model IDs.
Missing detection / guardrail: No validation that model IDs are whitespace-free after trimming.
Contributing context (if known): LLM-generated tool call parameters are the input source; they routinely break on long slash-delimited identifiers.

Regression Test Plan (if applicable)

Coverage level that should have caught this:
- Unit test
- Seam / integration test
- End-to-end test
- Existing coverage already sufficient
Target test or file: src/cron/normalize.test.ts
Scenario the test should lock in: normalizeCronJobCreate with model "custom-1/ minimax/ minimax- m2.5- free" and fallbacks containing internal spaces should output clean IDs with zero whitespace.
Why this is the smallest reliable guardrail: The corruption happens in coercePayload during normalization — a unit test on normalizeCronJobCreate catches it at the exact mutation point.
Existing test that already covers this (if any): Existing test only covered leading/trailing trim, not internal whitespace.
If no new test is added, why not: New test added.

User-visible / Behavior Changes

Model IDs with accidental internal whitespace are now silently corrected instead of being persisted as-is. No config changes.

Diagram (if applicable)

Before:
[LLM sends "custom-1/ minimax/ minimax-m2.5-free"] -> [coercePayload .trim()] -> [stored with spaces] -> [OpenRouter 400]

After:
[LLM sends "custom-1/ minimax/ minimax-m2.5-free"] -> [coercePayload .trim() + .replace(/\s+/g, '')] -> [stored clean] -> [OpenRouter OK]

Security Impact (required)

New permissions/capabilities? No
Secrets/tokens handling changed? No
New/changed network calls? No
Command/tool execution surface changed? No
Data access scope changed? No

Repro + Verification

Environment

OS: Windows 11 / Ubuntu 24 (reported)
Runtime/container: Node.js / Docker
Model/provider: OpenRouter → MiniMax
Integration/channel (if any): Cron → isolated session
Relevant config (redacted): custom-1/minimax/minimax-m2.5-free

Steps

Create cron job with model custom-1/minimax/minimax-m2.5-free
LLM inserts spaces: custom-1/ minimax/ minimax- m2.5- free
Job fails with 400 invalid model ID

Expected

Model ID stored as custom-1/minimax/minimax-m2.5-free

Actual

Model ID stored as custom-1/ minimax/ minimax- m2.5- free

Evidence

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)

53/53 tests pass including new "strips internal whitespace from model IDs" test.

Human Verification (required)

Verified scenarios: normalizeCronJobCreate with internal spaces in model and fallbacks fields
Edge cases checked: model with only leading/trailing spaces (existing test), model with multiple consecutive internal spaces, fallbacks array with spaces
What you did not verify: Live cron execution on a running OpenClaw instance

Review Conversations

I replied to or resolved every bot review conversation I addressed in this PR.
I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

Backward compatible? Yes
Config/env changes? No
Migration needed? No

Risks and Mitigations

Risk: Model IDs that intentionally contain spaces would be broken.
- Mitigation: No known provider uses spaces in model IDs. OpenRouter, Anthropic, Google, and OpenAI all use / and - delimiters without spaces.

greptile-apps · 2026-04-14T12:42:43Z

Greptile Summary

This PR fixes internal-whitespace corruption in cron job model IDs by applying .replace(/\s+/g, '') after the trim step in coercePayload and copyTopLevelAgentTurnFields. The fix is correct for the model field in both paths and for fallbacks in coercePayload, but the same stripping is missing for fallbacks in copyTopLevelAgentTurnFields (the legacy top-level input path), leaving that path still able to persist corrupted fallback IDs.

Confidence Score: 4/5

Fix is mostly correct but has one gap: top-level fallbacks in the legacy path skip whitespace stripping.
The model fix is complete across both code paths, but fallbacks whitespace stripping was only applied in coercePayload and not in copyTopLevelAgentTurnFields. Callers that pass top-level fallbacks (the legacy input format) will still persist corrupted entries. The issue is small and localized to one line, but it's the same class of bug the PR set out to fix.
src/cron/normalize.ts line 366 — payload.fallbacks = fallbacks in copyTopLevelAgentTurnFields needs .map(f => f.replace(/\s+/g, ''))

Comments Outside Diff (1)

src/cron/normalize.ts, line 363-368 (link)

Incomplete fix: top-level fallbacks not stripped

model gets whitespace stripped in both coercePayload and copyTopLevelAgentTurnFields, but fallbacks only gets stripped in coercePayload. When a caller passes fallbacks at the top level (the legacy path), copyTopLevelAgentTurnFields copies the entries without removing internal whitespace, leaving corrupted model IDs that will still cause OpenRouter 400s.

The existing test for this code path ("maps top-level model/thinking/timeout into legacy add params") doesn't exercise top-level fallbacks with internal spaces, so the gap isn't caught by the new test either.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/cron/normalize.ts
Line: 363-368

Comment:
**Incomplete fix: top-level `fallbacks` not stripped**

`model` gets whitespace stripped in both `coercePayload` and `copyTopLevelAgentTurnFields`, but `fallbacks` only gets stripped in `coercePayload`. When a caller passes `fallbacks` at the top level (the legacy path), `copyTopLevelAgentTurnFields` copies the entries without removing internal whitespace, leaving corrupted model IDs that will still cause OpenRouter 400s.



The existing test for this code path (`"maps top-level model/thinking/timeout into legacy add params"`) doesn't exercise top-level `fallbacks` with internal spaces, so the gap isn't caught by the new test either.

How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/cron/normalize.ts
Line: 363-368

Comment:
**Incomplete fix: top-level `fallbacks` not stripped**

`model` gets whitespace stripped in both `coercePayload` and `copyTopLevelAgentTurnFields`, but `fallbacks` only gets stripped in `coercePayload`. When a caller passes `fallbacks` at the top level (the legacy path), `copyTopLevelAgentTurnFields` copies the entries without removing internal whitespace, leaving corrupted model IDs that will still cause OpenRouter 400s.

```suggestion
  if (!Array.isArray(payload.fallbacks) && Array.isArray(next.fallbacks)) {
    const fallbacks = normalizeTrimmedStringArray(next.fallbacks);
    if (fallbacks !== undefined) {
      payload.fallbacks = fallbacks.map(f => f.replace(/\s+/g, ''));
    }
  }
```

The existing test for this code path (`"maps top-level model/thinking/timeout into legacy add params"`) doesn't exercise top-level `fallbacks` with internal spaces, so the gap isn't caught by the new test either.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix: strip internal whitespace from mode..." | Re-trigger Greptile}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 156cfcdd53

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-04-14T12:45:56Z

@@ -181,7 +181,7 @@ function coercePayload(payload: UnknownRecord) {
  if ("model" in next) {
    const model = parseOptionalField(TrimmedNonEmptyStringFieldSchema, next.model);
    if (model !== undefined) {
-      next.model = model;
+      next.model = model.replace(/\s+/g, '');


Keep spaces in alias model overrides

payload.model can be an alias (not just provider/model), but this replacement removes all whitespace, so an alias like "My Fast Model" is rewritten to "MyFastModel" and no longer matches alias resolution (which lowercases but preserves internal spaces). Since normalizeCronJobInput is also used when hydrating persisted jobs in src/cron/service/store.ts, existing cron jobs that rely on spaced aliases can start failing with invalid model after this change.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1b65e7e895

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-04-14T13:06:37Z

    const fallbacks = normalizeTrimmedStringArray(next.fallbacks);
    if (fallbacks !== undefined) {
-      next.fallbacks = fallbacks;
+      next.fallbacks = fallbacks?.map(f => f.replace(/\s+/g, '')) ?? fallbacks;


Preserve spaces in fallback aliases during normalization

Fallback entries can be alias strings, not just provider/model, and this normalization removes all internal whitespace from each fallback. An alias like "My Backup Model" is rewritten to "MyBackupModel", but alias matching in buildModelAliasIndex/resolveModelRefFromString lowercases while preserving internal spaces, so the fallback alias no longer resolves. Because normalizeCronJobInput is also used when reading persisted jobs in src/cron/service/store.ts, existing cron jobs with spaced fallback aliases can silently lose their fallback chain and fail instead of retrying on the intended model.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6ec67ebe88

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-04-14T13:15:36Z

    const normalized = normalizeOptionalString(value);
    if (normalized) {
-      payload[field] = normalized;
+      payload[field] = field === "model" ? normalized.replace(/\s+/g, '') : normalized;


Preserve spaces in top-level model aliases

copyTopLevelAgentTurnFields now strips all whitespace from top-level model values before moving them into payload.model, but top-level model hints are still a supported input path (for example in patch normalization) and aliases can legitimately contain internal spaces. Alias lookup lowercases but preserves internal spacing, so an alias like "My Fast Model" becomes "MyFastModel" here and no longer resolves, causing invalid model for updates/hydration that rely on the top-level compatibility path.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-04-14T13:15:36Z

    const fallbacks = normalizeTrimmedStringArray(next.fallbacks);
    if (fallbacks !== undefined) {
-      payload.fallbacks = fallbacks;
+      payload.fallbacks = fallbacks?.map(f => f.replace(/\s+/g, '')) ?? fallbacks;


Keep spaces when copying top-level fallback aliases

The top-level fallback compatibility path now applies .replace(/\s+/g, '') to each fallback entry, which breaks fallback aliases that include internal spaces. Fallback entries are resolved through the same alias machinery as primary model refs, where normalization trims and lowercases but does not remove inner spaces, so values like "My Backup Model" are rewritten to "MyBackupModel" and stop matching; jobs using top-level fallbacks patches then lose their intended fallback chain.

Useful? React with 👍 / 👎.

clawsweeper · 2026-04-27T04:45:00Z

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

Keep this PR open, but it is not merge-ready: the alias-preserving resolution-time direction is useful, yet the potential merge result still returns spaced slash-form refs unchanged in allow-any configs, leaves payload fallback refs on the raw fallback path, and has no real after-fix cron/provider proof.

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. Source inspection of current main and the potential merge result shows how spaced slash-form payload models and fallbacks flow through cron model selection into provider/fallback resolution without guaranteed compaction.

Is this the best way to solve the issue?

No. Resolution-time repair is the right boundary for preserving spaced aliases, but this PR is too narrow until allow-any success, fallback candidates, and real cron/provider proof are covered.

Security review:

Security review cleared: The diff changes TypeScript model-selection logic and tests only; no dependency, workflow, permission, secret-handling, network, or code-execution surface was added.

AGENTS.md: found and applied where relevant.

What I checked:

stale F-rated PR: PR was opened 2026-04-14T12:41:08Z, is older than 30 days, and the latest review rated it F.
proof blocker: real behavior proof is missing 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:

steipete: Authored the recent model-selection helper split and a cron normalization refactor touching the affected owner boundary. (role: recent model-selection and cron normalization contributor; confidence: high; commits: 310d2db31241, 934927fd13ed; files: src/agents/model-selection-shared.ts, src/agents/model-selection-resolve.ts, src/cron/normalize.ts)
vincentkoc: Authored the cron isolated runtime seam refactor around the run path that consumes payload model and fallback choices. (role: recent cron isolated-runtime contributor; confidence: medium; commits: 35176f3cb730; files: src/cron/isolated-agent/model-selection.ts, src/cron/isolated-agent/run-executor.ts, src/cron/isolated-agent/run-fallback-policy.ts)
rodrigouroz: Authored the earlier cron model-only update patch flow, which is adjacent to this PR's cron model override normalization surface. (role: adjacent cron model-patch contributor; confidence: medium; commits: 89dccc79a7b1; files: src/cron/normalize.ts, src/agents/tools/cron-tool.ts)

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

clawsweeper · 2026-04-29T16:13:17Z

Codex review: needs changes before merge.

What this changes:

The PR branch changes cron normalization to strip all internal whitespace from model and fallbacks values and adds a unit test for whitespace-corrupted provider/model IDs.

Required change before merge:

The bug is valid, the unsafe part of this PR is localized, and an automated replacement or branch repair can implement an alias-preserving fix with focused tests.

Review details

Best possible solution:

Revise or supersede this PR with an alias-preserving repair: keep model values exact after trim, resolve aliases first, and only repair accidental whitespace for clear provider/model refs when the raw value fails resolution. Cover payload and top-level create/patch paths for both primary model and fallbacks.

Acceptance criteria:

pnpm test src/cron/normalize.test.ts src/cron/isolated-agent.model-formatting.test.ts src/cron/isolated-agent/run.payload-fallbacks.test.ts src/agents/model-selection.test.ts src/agents/model-fallback.test.ts
pnpm check:changed

What I checked:

Current main trims only: coercePayload() stores parsed model and fallbacks after trim-only normalization, and copyTopLevelAgentTurnFields() copies top-level model and fallbacks after the same trim-only string/list normalization. (src/cron/normalize.ts:200, acae48b790fa)
Runtime does not collapse internal whitespace: Isolated cron model selection trims payload.model and passes it to resolveAllowedModelRef; payload fallbacks are returned as raw fallbacksOverride values for the fallback runner. (src/cron/isolated-agent/model-selection.ts:111, acae48b790fa)
Aliases preserve spaces: Alias indexing trims and lowercases aliases but preserves internal spaces; resolution checks the alias map before direct provider/model parsing. (src/agents/model-selection-shared.ts:343, acae48b790fa)
Docs support aliases with spaces: The models docs say models set accepts provider/model or an alias, and Hugging Face docs show aliases such as DeepSeek R1 and Qwen3 8B. Public docs: docs/providers/huggingface.md. (docs/providers/huggingface.md:152, acae48b790fa)
Persisted jobs are normalized on load: Cron store hydration calls normalizeCronJobInput(raw), so a blanket normalization change would rewrite existing persisted cron jobs in memory during load, including jobs using spaced aliases. (src/cron/service/store.ts:61, acae48b790fa)
PR diff is a blanket rewrite: The PR applies .replace(/\s+/g, '') to payload and top-level model and fallbacks, so it affects aliases as well as provider/model refs. (src/cron/normalize.ts:181, 6ec67ebe88f7)

Likely related people:

steipete: Recent history shows repeated maintenance of cron model rejection and shared model alias/fallback resolution, including commits for invalid cron payload models and alias-before-fallback behavior. (role: recent maintainer and model-selection owner; confidence: high; commits: 343f2d724593, aec5efed8d43, 310d2db31241; files: src/cron/isolated-agent/model-selection.ts, src/agents/model-selection-shared.ts, src/cron/normalize.ts)
vincentkoc: Recent commits touched cron normalization and isolated cron runtime seams, including flat legacy job row normalization and runtime-heavy seam extraction. (role: recent cron normalization maintainer; confidence: medium; commits: e0546edd9820, 35176f3cb730; files: src/cron/normalize.ts, src/cron/isolated-agent/model-selection.ts)

Remaining risk / open question:

Landing this PR as-is would rewrite valid spaced primary aliases before alias lookup.
Landing this PR as-is would rewrite valid spaced fallback aliases and could remove the intended fallback chain.
Because cron store hydration normalizes persisted jobs, the PR could affect existing jobs on load, not only newly created jobs.

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

Revert blanket internal-whitespace collapse from normalize and instead apply it lazily at resolution time in resolveAllowedModelRefFromAliasIndex. The repair only fires for slash-form refs (provider/model) and only when the verbatim trimmed value fails alias lookup or allowlist check, so spaced aliases such as 'DeepSeek R1' are never mutated.

clawsweeper · 2026-05-20T19:15:32Z

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.

RomneyDa · 2026-05-29T06:35:59Z

Heads up: this PR needs to be updated against current main before the new required Dependency Guard check can pass.

openclaw-barnacle · 2026-06-13T04:59:44Z

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

clawsweeper · 2026-06-13T05:07:54Z

ClawSweeper applied the proposed close for this PR.

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

openclaw-barnacle Bot added the size: XS label Apr 14, 2026

chatgpt-codex-connector Bot reviewed Apr 14, 2026

View reviewed changes

Angfr95 force-pushed the fix/cron-model-id-whitespace branch 2 times, most recently from 65b1f96 to 1b65e7e Compare April 14, 2026 13:02

chatgpt-codex-connector Bot reviewed Apr 14, 2026

View reviewed changes

steipete mentioned this pull request Apr 25, 2026

[Bug]: Cron Tool Corrupts Model IDs #66203

Closed

openclaw-clownfish Bot mentioned this pull request Apr 29, 2026

fix(cron): preserve model overrides for text payloads #73946

Merged

openclaw-barnacle Bot added channel: line Channel integration: line channel: slack Channel integration: slack app: web-ui App: web-ui gateway Gateway runtime commands Command implementations channel: feishu Channel integration: feishu size: M and removed size: XS labels Apr 29, 2026

fix: strip internal whitespace from model IDs in cron job normalization

c8c782b

Angfr95 force-pushed the fix/cron-model-id-whitespace branch from 9c5aa59 to c8c782b Compare April 29, 2026 20:41

openclaw-barnacle Bot added size: XS and removed channel: line Channel integration: line channel: slack Channel integration: slack app: web-ui App: web-ui gateway Gateway runtime commands Command implementations channel: feishu Channel integration: feishu size: M labels Apr 29, 2026

openclaw-barnacle Bot added agents Agent runtime and tooling size: S and removed size: XS labels Apr 29, 2026

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

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

clawsweeper Bot closed this Jun 13, 2026

vincentkoc mentioned this pull request Jun 16, 2026

fix(cron): preserve model overrides for text payloads #93527

Merged

Uh oh!

Uh oh!

Conversation

Angfr95 commented Apr 14, 2026

Summary

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Root Cause (if applicable)

Regression Test Plan (if applicable)

User-visible / Behavior Changes

Diagram (if applicable)

Security Impact (required)

Repro + Verification

Environment

Steps

Expected

Actual

Evidence

Human Verification (required)

Review Conversations

Compatibility / Migration

Risks and Mitigations

Uh oh!

greptile-apps Bot commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Comments Outside Diff (1)

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 14, 2026

Choose a reason for hiding this comment

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 14, 2026

Choose a reason for hiding this comment

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 14, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Apr 14, 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

Uh oh!

RomneyDa commented May 29, 2026

Uh oh!

openclaw-barnacle Bot commented Jun 13, 2026

Uh oh!

clawsweeper Bot commented Jun 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

greptile-apps Bot commented Apr 14, 2026 •

edited

Loading

clawsweeper Bot commented Apr 27, 2026 •

edited

Loading