fix: respect replace mode in default models list by TUARAN · Pull Request #94825 · openclaw/openclaw

TUARAN · 2026-06-19T06:29:23Z

Summary

make default openclaw models list stop appending auth-backed built-in catalogs when models.mode is "replace" and the config does not opt into provider wildcard visibility
keep replace-mode wildcard behavior narrow by allowing dynamic catalog rows only for providers explicitly allowed via agents.defaults.models["provider/*"]
add focused regression coverage for both replace-mode paths

Verification

node scripts/run-vitest.mjs src/commands/models.list.e2e.test.ts
git diff --check
Real behavior proof:
- Behavior addressed: openclaw models list --json no longer shows unrelated auth-backed providers when models.mode is "replace" with a restricted provider config.
- Real environment tested: source-checkout OpenClaw CLI command path via node --import tsx src/entry.ts models list --json on June 19, 2026 with a temporary OPENCLAW_CONFIG_PATH, OPENCLAW_HOME, and OPENCLAW_STATE_DIR.
- Exact steps or command run after this patch: OPENCLAW_HOME="$home" OPENCLAW_STATE_DIR="$state" OPENAI_API_KEY=test-openai GROQ_API_KEY=test-groq DEEPSEEK_API_KEY=test-deepseek XAI_API_KEY=test-xai OPENCLAW_CONFIG_PATH="$config" node --import tsx src/entry.ts models list --json
- Evidence after fix: terminal JSON output reported count: 3 with keys deepseek/deepseek-v4-pro, deepseek/deepseek-v4-flash, and xai/grok-4.3; no openai/* or groq/* rows were present.
- Observed result after fix: the default list matched the replace-mode allowlist/configured-provider scope instead of leaking other authenticated built-in providers into the visible model list.
- What was not tested: packaged dist launcher output, --all behavior, and wildcard-provider CLI evidence outside the focused unit regression coverage.

clawsweeper · 2026-06-19T06:32:11Z

Codex review: needs changes before merge. Reviewed June 21, 2026, 9:37 AM ET / 13:37 UTC.

Summary
The PR adds a replace-mode guard to appendAuthenticatedCatalogRows so default openclaw models list only appends authenticated catalog rows for wildcard-allowlisted providers, plus two regression tests.

PR surface: Source +22, Tests +123. Total +145 across 2 files.

Reproducibility: yes. Source inspection shows current main's default models list path appends authenticated catalog rows without checking models.mode, and the linked issue supplies concrete replace-mode leaked-provider output.

Review metrics: 1 noteworthy metric.

Existing Config-Mode Output: 1 changed. The PR changes how models.mode=replace affects default models list visibility, so maintainers should notice the upgrade behavior before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #94705
Summary: This PR is a candidate fix for the canonical replace-mode default model-list leak; nearby open PRs target the same symptom or adjacent replace-mode provider-discovery behavior.

Members:

canonical: models.mode: "replace" does not filter openclaw models list output #94705 - The issue contains the concrete replace-mode openclaw models list reproduction and is closed by this PR if merged.
same_root_cause: fix(models): respect models.mode=replace in 'openclaw models list' output #94735 - That open PR also uses closing syntax for the same issue and changes model-list replace-mode behavior, but prior review found explicit-browse compatibility and proof blockers.
same_root_cause: fix(models): filter models list by configured providers in replace mode [AI-assisted] #94914 - That open PR targets the same replace-mode list filtering symptom, but prior review found a broader shared row-filter compatibility issue and missing real behavior proof.
superseded: fix(models): respect models.mode: "replace" in models list output #94722 - That closed unmerged PR attempted the same replace-mode model-list fix and now only preserves useful context.
partial_overlap: fix(agents): skip implicit provider discovery when models.mode is 'replace' [AI-assisted] #82638 - That open PR addresses adjacent replace-mode implicit provider discovery in agent model config planning, not the default CLI list output leak fixed here.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🦞 diamond lobster
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:

Type the loadModelCatalog mock or fixture rows so check-test-types passes.
[P2] Rerun node scripts/run-vitest.mjs src/commands/models.list.e2e.test.ts after the type repair.

Risk before merge

[P1] Exact PR head currently fails check-test-types on the new catalog fixture rows, so the branch is not merge-ready even though the runtime change is narrow.
[P1] Existing replace-mode users or scripts that consumed leaked authenticated catalog rows from the default list will see narrower output after upgrade; this matches the documented contract but is still compatibility-sensitive.
[P1] Multiple same-root candidate PRs remain open, so maintainers should land one canonical fix and then close or redirect the others.

Maintainer options:

Repair Test Typing, Then Land The Narrow Fix (recommended)
Fix the new test mock typing and land if maintainers accept that default replace-mode listing should hide non-allowlisted auth-backed rows.
Ask For Packaged CLI Proof
Pause after the type repair if maintainers want the same replace-mode filtering demonstrated through the packaged launcher rather than the source-checkout command path.
Keep Legacy Leaked Default Rows
Close or redirect this PR if maintainers decide existing replace-mode default output should continue showing authenticated built-in providers despite the docs.

Copy recommended automerge instruction

@clawsweeper automerge

Special instructions:
Fix the `src/commands/models.list.e2e.test.ts` `loadModelCatalog` mock typing so the inserted catalog fixture rows satisfy `pnpm check:test-types`, without changing the runtime `appendAuthenticatedCatalogRows` behavior or broadening replace-mode filtering beyond the default list path.

Next step before merge

[P2] Queue a narrow repair because the remaining actionable blocker is mechanical test mock typing; maintainer merge judgment can happen after green checks.

Security
Cleared: The diff only changes TypeScript CLI row selection and tests; it adds no dependency, workflow, secret-handling, package, install, or release surface.

Review findings

[P1] Type the catalog mock rows — src/commands/models.list.e2e.test.ts:675

Review details

Best possible solution:

Preserve the default-list-only replace-mode gate, type the new catalog fixtures against the real catalog row shape, and land one canonical fix for #94705 after checks pass.

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

Yes. Source inspection shows current main's default models list path appends authenticated catalog rows without checking models.mode, and the linked issue supplies concrete replace-mode leaked-provider output.

Is this the best way to solve the issue?

Yes for the runtime fix shape, but not as a mergeable branch yet. The patch scopes filtering to the default authenticated catalog append and preserves explicit browse paths, but the new tests need a type repair before this can land.

Full review comments:

[P1] Type the catalog mock rows — src/commands/models.list.e2e.test.ts:675
The new tests call loadModelCatalog.mockResolvedValue([...]) while the mock is declared as vi.fn(async () => []), so tsgo infers the resolved array as never[]. Exact-head check-test-types fails with TS2322 on these inserted catalog rows; give the mock or fixtures the real catalog return type so the test type lane passes.
Confidence: 0.94

Overall correctness: patch is incorrect
Overall confidence: 0.91

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P2: The PR targets a real but limited models-list provider visibility bug; runtime model allowlisting still blocks disallowed selections.
merge-risk: 🚨 compatibility: Default models list output for existing replace-mode configurations becomes narrower after upgrade, even though that aligns with the documented contract.
rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦞 diamond lobster and patch quality is 🧂 unranked krab.
status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR body includes copied source-checkout CLI output with temporary config/home/state and after-fix JSON keys excluding unrelated authenticated providers.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied source-checkout CLI output with temporary config/home/state and after-fix JSON keys excluding unrelated authenticated providers.

Evidence reviewed

PR surface:

Source +22, Tests +123. Total +145 across 2 files.

View PR surface stats

Area	Files	Added	Net
Source	1	22	+22
Tests	1	123	+123
Docs	0	0	0
Config	0	0	0
Generated	0	0	0
Other	0	0	0
Total	2	145	+145

Acceptance criteria:

[P1] pnpm check:test-types.
[P1] node scripts/run-vitest.mjs src/commands/models.list.e2e.test.ts.
[P1] git diff --check.

What I checked:

Repository policy applied: Root AGENTS.md was read fully; its PR review and compatibility guidance applies because this PR changes model/provider visibility for an existing config mode. (AGENTS.md:12, 11a2e03bd4de)
No matching scoped policy or maintainer note: No src/commands/AGENTS.md exists and .agents/maintainer-notes/telegram.md is unrelated to the models-list command surface.
Current default-list entry point: Plain default models list does not enable the all/provider source-plan cascade and instead calls appendConfiguredModelRowSources. (src/commands/models/list.list-command.ts:118, 11a2e03bd4de)
Current main leak path: appendConfiguredModelRowSources appends configured rows and then unconditionally calls appendAuthenticatedCatalogRows on current main. (src/commands/models/list.row-sources.ts:181, 11a2e03bd4de)
Current authenticated catalog behavior: Current main loads the catalog and filters by provider auth, but it has no models.mode or configured-provider guard before appending rows. (src/commands/models/list.rows.ts:390, 11a2e03bd4de)
Documented replace-mode contract: Config help says models.mode: "replace" uses only configured providers, supporting this as a bug fix rather than a new feature. (src/config/schema.help.ts:953, 11a2e03bd4de)

Likely related people:

steipete: GitHub path history ties this account to adding authenticated catalog rows in configured lists, restoring provider catalog listing, and documenting model-list source behavior. (role: model-list architecture contributor; confidence: high; commits: d5c094f1691a, 56c4f9761c75, 9d7f83b17504; files: src/commands/models/list.row-sources.ts, src/commands/models/list.rows.ts, docs/cli/models.md)
shakkernerd: GitHub history shows source-plan and provider-filter model-list fixes that define the explicit browse behavior this PR avoids changing. (role: source-plan and provider-filter contributor; confidence: high; commits: 25dda844b710, 4e4f9204d75c, b418c08a2297; files: src/commands/models/list.source-plan.ts, src/commands/models/list.row-sources.ts, src/commands/models/list.list-command.ts)
samson910022: Recent merged work bounded default model browsing while preserving explicit full-catalog browsing, which is the compatibility boundary this PR relies on. (role: adjacent model browse contributor; confidence: medium; commits: 44f45d8729f5; files: src/commands/models/list.rows.ts, src/commands/models/list.rows.test.ts)
vincentkoc: Current-main blame on the row-source path and authenticated catalog row builder points to a recent Vincent Koc-authored forward-port/refactor commit in the central files. (role: recent area contributor; confidence: medium; commits: 61d1fd1f7217, 6c83e8e7e419; files: src/commands/models/list.row-sources.ts, src/commands/models/list.rows.ts, src/commands/models/list.source-plan.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.

steipete · 2026-07-09T14:50:43Z

Thanks @TUARAN. Closing this older implementation while keeping #94705 and the newer owner-assigned candidate #94735 as the canonical path. This head lacks current proof and no longer represents the preferred fix, so it has no distinct change to land.

fix(models): respect replace mode default list

eea94f6

openclaw-barnacle Bot added commands Command implementations size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 19, 2026

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

openclaw-barnacle Bot added the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 21, 2026

steipete closed this Jul 9, 2026

clawsweeper Bot mentioned this pull request Jul 9, 2026

fix(models): honor replace mode in list output #103103

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix: respect replace mode in default models list#94825

fix: respect replace mode in default models list#94825
TUARAN wants to merge 1 commit into
openclaw:mainfrom
TUARAN:codex/issue-94705-models-list

TUARAN commented Jun 19, 2026

Uh oh!

clawsweeper Bot commented Jun 19, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

TUARAN commented Jun 19, 2026

Uh oh!

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

Uh oh!

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented Jun 19, 2026 •

edited

Loading