fix(doctor): migrate invalid thinking formats by giodl73-repo · Pull Request #84626 · openclaw/openclaw

giodl73-repo · 2026-05-20T16:23:07Z

Summary

openclaw doctor --fix now removes unrecognized models.providers.*.models[*].compat.thinkingFormat values from provider model entries so stale model config can validate after upgrade. The migration is provider-generic: it was reported with Bailian config, but the stored config shape is shared by all configured model providers.

This also centralizes the supported thinkingFormat literals in src/config/types.models.ts and reuses that contract from:

config schema validation
model catalog normalization
doctor legacy migration detection and repair

Behavior addressed

doctor --fix previously detected the invalid config but had no migration for stale compat.thinkingFormat values, so writeback could still fail validation. After this patch, only unrecognized optional compat.thinkingFormat values are deleted and the rest of compat is preserved.

Real environment tested

Local WSL source checkout, Node 24, with a temporary OPENCLAW_CONFIG_PATH and OPENCLAW_STATE_DIR.

Exact steps or command run after this patch

tmp=$(mktemp -d /tmp/openclaw-77803-proof.XXXXXX)
cat > "$tmp/openclaw.json" <<'JSON'
{
  "models": {
    "providers": {
      "bailian": {
        "baseUrl": "https://dashscope.aliyuncs.com/compatible-mode/v1",
        "api": "openai-completions",
        "models": [
          {
            "id": "qwen-legacy",
            "name": "Qwen Legacy",
            "compat": {
              "thinkingFormat": "bailian-legacy",
              "supportsTools": true
            }
          },
          {
            "id": "qwen-valid",
            "name": "Qwen Valid",
            "compat": {
              "thinkingFormat": "qwen"
            }
          }
        ]
      }
    }
  }
}
JSON
OPENCLAW_CONFIG_PATH="$tmp/openclaw.json" OPENCLAW_STATE_DIR="$tmp/state" pnpm openclaw doctor --fix
cat "$tmp/openclaw.json"

Evidence after fix

openclaw doctor --fix printed:

Legacy config keys detected
- models.providers:
  models.providers.<id>.models[*].compat.thinkingFormat has an unrecognized value; run "openclaw doctor --fix" to remove it and restore the runtime default.

Doctor changes
Removed models.providers.bailian.models.0.compat.thinkingFormat (unrecognized value "bailian-legacy"; runtime default applies).

Updated config: /tmp/openclaw-77803-proof.ozhQe7/openclaw.json
Doctor complete.

The repaired config preserved supportsTools: true, removed only the invalid "bailian-legacy" value, and preserved the valid "qwen" value:

{
  "id": "qwen-legacy",
  "name": "Qwen Legacy",
  "compat": {
    "supportsTools": true
  }
}

{
  "id": "qwen-valid",
  "name": "Qwen Valid",
  "compat": {
    "thinkingFormat": "qwen"
  }
}

Observed result after fix

The real doctor --fix CLI path completed successfully and wrote the migrated config. The invalid compat.thinkingFormat no longer blocks validation/writeback, while valid values remain intact.

What was not tested

I did not reproduce on the reporter's Windows 10 host or with their exact private config, because the issue did not include the literal old Bailian value. The test and CLI proof exercise the same persisted config path with a representative invalid literal.

Verification

pnpm install --frozen-lockfile
pnpm exec oxfmt --write --threads=1 src/config/types.models.ts src/config/zod-schema.core.ts src/model-catalog/normalize.ts src/commands/doctor/shared/legacy-config-migrations.runtime.models.ts src/commands/doctor/shared/legacy-config-migrations.runtime.ts src/commands/doctor/shared/legacy-config-migrate.test.ts src/commands/doctor/shared/legacy-config-migrate.validation.test.ts CHANGELOG.md
node scripts/run-vitest.mjs src/commands/doctor/shared/legacy-config-migrate.test.ts src/commands/doctor/shared/legacy-config-migrate.validation.test.ts src/config/config-misc.test.ts src/model-catalog/normalize.test.ts --reporter=dot
node scripts/run-vitest.mjs src/commands/doctor/shared/legacy-config-migrate.test.ts src/commands/doctor/shared/legacy-config-migrate.validation.test.ts --reporter=dot
git diff --check
/mnt/c/src/claws-hapi/.agents/skills/autoreview/scripts/autoreview --mode local

Results:

Focused Vitest: 4 files passed, 140 tests passed.
Focused migration Vitest rerun: 2 files passed, 56 tests passed.
Real CLI doctor --fix proof passed and wrote the repaired config.
git diff --check passed.
Autoreview initially found a real type-narrowing issue in model catalog normalization; fixed with a shared isModelThinkingFormat type guard. Rerun was clean: no accepted/actionable findings.

Additional typecheck note:

node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.test.src.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/test-src-doctor-thinking-format-77803.tsbuildinfo

This currently fails on origin/main-adjacent status test data, unrelated to this patch:

src/commands/status.summary.redaction.test.ts(70,18): missing configuredModel, selectedModel, modelSelectionReason
src/commands/status.summary.redaction.test.ts(76,22): missing configuredModel, selectedModel, modelSelectionReason

clawsweeper · 2026-05-20T16:25:36Z

Codex review: needs maintainer review before merge.

Workflow note: Future ClawSweeper reviews update this same comment in place.

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.

Summary
Adds a doctor legacy-config migration for unrecognized models.providers.*.models[*].compat.thinkingFormat values, centralizes the supported literals, reuses that guard in schema/catalog normalization, and updates tests plus changelog.

Reproducibility: yes. Current main rejects unrecognized compat.thinkingFormat values and has no registered models legacy migration before validation; I verified this by source inspection rather than rerunning the current-main CLI path.

PR rating
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Summary: Strong proof and a focused implementation with no blocking findings; merge readiness depends on normal maintainer and CI handling.

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.

Real behavior proof
Sufficient (terminal): The PR body includes after-fix terminal proof from the real openclaw doctor --fix CLI path using temporary config/state paths and showing the repaired config.

Risk before merge

One GitHub check run was still in progress on head 61a6f737f957a836a3f97742aab3ed2603e59a00 at inspection, so final merge should wait for required checks to settle.
The reporter's exact private Windows/Bailian config value is unavailable; the PR proof uses a representative invalid literal on the same persisted config path.

Maintainer options:

Decide the mitigation before merge
Land the focused migration after required checks and maintainer review, keeping the shared MODEL_THINKING_FORMATS guard as the source of truth and closing the linked bug on merge.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge
No repair lane is needed; the patch is focused with sufficient proof and no blocking findings, and the remaining work is maintainer/CI merge handling.

Security
Cleared: The diff is limited to config migration/schema/catalog code, focused tests, and changelog text with no concrete security or supply-chain concern.

Review details

Best possible solution:

Land the focused migration after required checks and maintainer review, keeping the shared MODEL_THINKING_FORMATS guard as the source of truth and closing the linked bug on merge.

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

Yes. Current main rejects unrecognized compat.thinkingFormat values and has no registered models legacy migration before validation; I verified this by source inspection rather than rerunning the current-main CLI path.

Is this the best way to solve the issue?

Yes. Removing only invalid optional thinkingFormat values through doctor --fix is narrow, provider-generic, and the shared guard keeps schema/catalog/migration behavior aligned.

Label justifications:

P2: This is a normal-priority doctor/config migration bug fix with limited blast radius, regression tests, and real CLI proof.
rating: 🦞 diamond lobster: Current PR rating is 🦞 diamond lobster because proof is 🦞 diamond lobster, patch quality is 🦞 diamond lobster, and Strong proof and a focused implementation with no blocking findings; merge readiness depends on normal maintainer and CI handling.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix terminal proof from the real openclaw doctor --fix CLI path using temporary config/state paths and showing the repaired config.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal proof from the real openclaw doctor --fix CLI path using temporary config/state paths and showing the repaired config.

What I checked:

current_main_rejects_unknown_thinking_format: Current main validates compat.thinkingFormat against a fixed literal set, so stale unrecognized persisted values fail config validation. (src/config/zod-schema.core.ts:205, d5cc0d53b7e3)
current_main_has_no_models_runtime_migration: Current main registers runtime legacy migrations for several areas but has no models migration, leaving invalid model compat values unrepaired before post-migration validation. (src/commands/doctor/shared/legacy-config-migrations.runtime.ts:10, d5cc0d53b7e3)
pr_adds_targeted_migration: The PR adds LEGACY_CONFIG_MIGRATIONS_RUNTIME_MODELS, detects invalid string thinkingFormat values, deletes only that field, and records one change per repaired model entry. (src/commands/doctor/shared/legacy-config-migrations.runtime.models.ts:39, 61a6f737f957)
shared_literal_guard: The PR moves the supported literal list into MODEL_THINKING_FORMATS and reuses isModelThinkingFormat from schema, catalog normalization, and the doctor migration, reducing drift across these paths. (src/config/types.models.ts:53, 61a6f737f957)
real_cli_proof_in_pr_body: The PR body includes after-fix openclaw doctor --fix terminal output using temporary config/state paths; it shows the invalid Bailian representative value removed, supportsTools preserved, and valid qwen preserved. (61a6f737f957)
related_prior_attempt: Search found the linked bug, this active PR, and an earlier closed unmerged attempt, so this PR is the active implementation path rather than a duplicate of an open canonical PR.

Likely related people:

indulgeback: Commit c91fffd added Qwen thinkingFormat values across config schema, model catalog normalization, docs, and runtime mapping, which is the schema surface this PR repairs around. (role: introduced thinkingFormat behavior; confidence: high; commits: c91fffdd67d6; files: src/config/types.models.ts, src/config/zod-schema.core.ts, src/model-catalog/normalize.ts)
steipete: Commit history shows recent work splitting and organizing doctor runtime migrations, and the current PR is registered in that same migration list. (role: recent doctor migration area contributor; confidence: high; commits: 2ff29a33d0e9; files: src/commands/doctor/shared/legacy-config-migrations.runtime.ts, src/commands/doctor/shared/legacy-config-migrate.ts)
hclsys: Commit b1db87f changed doctor --fix migration persistence when validation problems remain, which is adjacent to this PR's stale-config writeback behavior. (role: doctor --fix validation-path contributor; confidence: medium; commits: b1db87fb3646; files: src/commands/doctor/shared/legacy-config-migrate.ts, src/commands/doctor/shared/legacy-config-migrate.validation.test.ts)
vincentkoc: Commit 9ca98a6 recently updated accepted thinking-format literals, making this contributor relevant to the shared literal contract. (role: recent thinkingFormat contributor; confidence: medium; commits: 9ca98a6d399b; files: src/config/types.models.ts)

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

clawsweeper · 2026-05-20T16:30:53Z

ClawSweeper PR egg

✨ Hatched: 🥚 common Mossy Lint Imp

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

Merged PRs are hatchable.
Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.

Rarity: 🥚 common.
Trait: sparkles near resolved comments.
Image traits: location status garden; accessory proof snapshot camera; palette amber, ink, and glacier blue; mood focused; pose waving from a small platform; shell polished stone shell; lighting warm desk-lamp glow; background little resolved-comment flags.
Share on X: post this hatch
Copy: My PR egg hatched a 🥚 common Mossy Lint Imp in ClawSweeper.

What is this egg doing here?

Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

@altaywtf

* fix(errors): dedupe identical messages when traversing error .cause chain (openclaw#84556) Merged via squash. Prepared head SHA: 46aa27f Co-authored-by: RomneyDa <6581799+RomneyDa@users.noreply.github.com> Co-authored-by: altaywtf <9790196+altaywtf@users.noreply.github.com> Reviewed-by: @altaywtf * fix(cli): gate exported subcli descriptors (openclaw#84519) Summary: - This PR filters exported sub-CLI descriptors through the private-QA gate, centralizes that filter, adds regr ... ge, and carries small validation repairs in workspace glob and tunnel-timeout tests plus a changelog entry. - Reproducibility: yes. Current-main source shows the raw SUB_CLI_DESCRIPTORS export can include qa while the helper surfaces filter it, and src/cli/argv.ts consumes that export for root command policy. Automerge notes: - PR branch already contained follow-up commit before automerge: fix(cli): gate exported subcli descriptors - PR branch already contained follow-up commit before automerge: fix(clawsweeper): address review for automerge-openclaw-openclaw-8451… Validation: - ClawSweeper review passed for head ba197a6. - Required merge gates passed before the squash merge. Prepared head SHA: ba197a6 Review: openclaw#84519 (comment) Co-authored-by: Zhaocun <zhaocunsun@gmail.com> Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <781889+takhoffman@users.noreply.github.com> * fix(doctor): migrate invalid thinking formats (openclaw#84626) * fix(cron-cli): bound loadCronJobForShow pagination (openclaw#83856) (openclaw#83989) Summary: - Adds a 50-page and advancing-`nextOffset` guard to `loadCronJobForShow`, exports that helper for regression tests, and adds an unreleased changelog entry. - Reproducibility: yes. Current main is source-reproducible because `loadCronJobForShow` loops while `hasMore` ... ed numeric `nextOffset`; the PR discussion also includes terminal before/after proof for the same CLI path. Automerge notes: - No ClawSweeper repair was needed after automerge opt-in. Validation: - ClawSweeper review passed for head 7828b4b. - Required merge gates passed before the squash merge. Prepared head SHA: 7828b4b Review: openclaw#83989 (comment) Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <781889+takhoffman@users.noreply.github.com> * fix(config): accept execApprovals.enabled="auto" in zod schema * fix: honour tool error suppression for mutating tools (openclaw#81561) Merged via squash. Prepared head SHA: 7462a86 Co-authored-by: moeedahmed <5780040+moeedahmed@users.noreply.github.com> Co-authored-by: jalehman <550978+jalehman@users.noreply.github.com> Reviewed-by: @jalehman * Add OpenRouter provider routing params (openclaw#84579) Co-authored-by: Alex Knight <15041791+amknight@users.noreply.github.com> * Preserve AGENTS.md policy during bootstrap truncation (openclaw#82921) Fixes openclaw#82920 * chore: regenerate base config schema Updated after MODEL_THINKING_FORMATS changed from z.union literals to z.enum, and session/session.agentToAgent gained detailed help text. * Revert "Add OpenRouter provider routing params (openclaw#84579)" This reverts commit 53254dc. --------- Co-authored-by: Dallin Romney <dallinromney@gmail.com> Co-authored-by: RomneyDa <6581799+RomneyDa@users.noreply.github.com> Co-authored-by: altaywtf <9790196+altaywtf@users.noreply.github.com> Co-authored-by: Zhaocun Sun <zhaocunsun@gmail.com> Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: takhoffman <781889+takhoffman@users.noreply.github.com> Co-authored-by: Gio Della-Libera <giodl73@gmail.com> Co-authored-by: yaoyi1222 <yaoyi_1222@163.com> Co-authored-by: Sarah Fortune <sarah.fortune@gmail.com> Co-authored-by: Moeed Ahmed <drmoeedahmed@gmail.com> Co-authored-by: moeedahmed <5780040+moeedahmed@users.noreply.github.com> Co-authored-by: jalehman <550978+jalehman@users.noreply.github.com> Co-authored-by: Alex Knight <aknight@atlassian.com> Co-authored-by: Alex Knight <15041791+amknight@users.noreply.github.com> Co-authored-by: Galin Iliev <iliev@galcho.com>

openclaw-barnacle Bot added commands Command implementations size: M maintainer Maintainer-authored PR labels May 20, 2026

giodl73-repo marked this pull request as ready for review May 20, 2026 17:03

simonvanlaak mentioned this pull request May 20, 2026

CI: Blacksmith-backed PR checks remain queued with no runner assigned #84663

Closed

giodl73-repo assigned steipete May 20, 2026

giodl73-repo force-pushed the fix-doctor-thinking-format-77803 branch from f6220f5 to 47b52b7 Compare May 20, 2026 20:18

fix(doctor): migrate invalid thinking formats

61a6f73

giodl73-repo force-pushed the fix-doctor-thinking-format-77803 branch from 47b52b7 to 61a6f73 Compare May 20, 2026 20:23

giodl73-repo merged commit 6e9d47b into openclaw:main May 20, 2026
98 checks passed

cubic-dev-ai Bot mentioned this pull request May 20, 2026

[pull] main from openclaw:main Jahid11978/openclaw-2d108f14#143

Merged

github-actions Bot mentioned this pull request May 20, 2026

📡 Upstream Digest — 2026-05-20 21:44 UTC curtismercier/openclaw-mods#904

Open

frankhli843 mentioned this pull request May 21, 2026

maintenance: selective upstream sync 2026-05-21 (8 commits ported) gemmaclaw/gemmaclaw#209

Merged

4 tasks

SebTardif pushed a commit to SebTardif/openclaw that referenced this pull request May 24, 2026

fix(doctor): migrate invalid thinking formats (openclaw#84626)

68b9786

SebTardif pushed a commit to SebTardif/openclaw that referenced this pull request May 24, 2026

fix(doctor): migrate invalid thinking formats (openclaw#84626)

de98184

SebTardif pushed a commit to SebTardif/openclaw that referenced this pull request May 24, 2026

fix(doctor): migrate invalid thinking formats (openclaw#84626)

1e02fbd

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 24, 2026

fix(doctor): migrate invalid thinking formats (openclaw#84626)

a2037c5

galiniliev pushed a commit to galiniliev/openclaw that referenced this pull request May 25, 2026

fix(doctor): migrate invalid thinking formats (openclaw#84626)

142f1d2

SebTardif pushed a commit to SebTardif/openclaw that referenced this pull request May 26, 2026

fix(doctor): migrate invalid thinking formats (openclaw#84626)

502bb2b

SebTardif pushed a commit to SebTardif/openclaw that referenced this pull request May 26, 2026

fix(doctor): migrate invalid thinking formats (openclaw#84626)

996934d

SebTardif pushed a commit to SebTardif/openclaw that referenced this pull request May 26, 2026

fix(doctor): migrate invalid thinking formats (openclaw#84626)

dd4fafd

jameslcowan pushed a commit to jameslcowan/openclaw that referenced this pull request Jun 2, 2026

fix(doctor): migrate invalid thinking formats (openclaw#84626)

fed602e

SYU8384 pushed a commit to SYU8384/openclaw that referenced this pull request Jun 3, 2026

fix(doctor): migrate invalid thinking formats (openclaw#84626)

c0f7b59

sablehead pushed a commit to sablehead/openclaw that referenced this pull request Jun 10, 2026

fix(doctor): migrate invalid thinking formats (openclaw#84626)

ec7a880

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(doctor): migrate invalid thinking formats#84626

fix(doctor): migrate invalid thinking formats#84626
giodl73-repo merged 1 commit into
openclaw:mainfrom
giodl73-repo:fix-doctor-thinking-format-77803

giodl73-repo commented May 20, 2026

Uh oh!

clawsweeper Bot commented May 20, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

giodl73-repo commented May 20, 2026

Summary

Behavior addressed

Real environment tested

Exact steps or command run after this patch

Evidence after fix

Observed result after fix

What was not tested

Verification

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!

clawsweeper Bot commented May 20, 2026

Hatch command

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented May 20, 2026 •

edited

Loading