fix(agent): use static catalog for embedded model fast path#85696
fix(agent): use static catalog for embedded model fast path#85696wyf027 wants to merge 1 commit into
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed June 15, 2026, 6:14 AM ET / 10:14 UTC. Summary PR surface: Source +2, Tests +46. Total +48 across 4 files. Reproducibility: no. high-confidence live reproduction was run in this read-only review. The linked issue has concrete shipped-release timing logs, and current source shows native Moonshot can miss the lean pass and fall into broad model discovery. Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Land a narrowed fix that gives native Moonshot a lean static-catalog resolution path while keeping auth/config-aware dynamic hooks first unless a provider explicitly opts into static-first metadata, with redacted before/after dispatch timing proof. Do we have a high-confidence way to reproduce the issue? No high-confidence live reproduction was run in this read-only review. The linked issue has concrete shipped-release timing logs, and current source shows native Moonshot can miss the lean pass and fall into broad model discovery. Is this the best way to solve the issue? No as merge-ready: using bundled static catalog metadata is a plausible Moonshot fix, but this branch makes a broader provider precedence change and lacks real dispatch timing proof. A narrower provider-owned or capability-gated path is safer. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 0314819f918a. Label changesLabel justifications:
Evidence reviewedPR surface: Source +2, Tests +46. Total +48 across 4 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0b6fb0a9c
ℹ️ 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".
| const staticCatalogRuntimeHooks = options?.skipPiDiscovery | ||
| ? STATIC_PROVIDER_RUNTIME_HOOKS | ||
| : runtimeHooks; |
There was a problem hiding this comment.
Preserve provider model normalization on static fast path
When skipPiDiscovery is enabled, this path forces STATIC_PROVIDER_RUNTIME_HOOKS for static-catalog resolutions, so normalizeResolvedModel(...) no longer runs provider normalizeResolvedModel hooks for those models. That regresses providers that rely on normalization to attach runtime compat metadata (for example Mistral’s hook in extensions/mistral/index.ts applies applyMistralModelCompat, which sets compat.supportsReasoningEffort/reasoningEffortMap for mistral-medium-3-5 and mistral-small-latest); without that metadata, downstream OpenAI-compat detection falls back to supportsReasoningEffort: false, so embedded-runner fast-path requests can silently drop reasoning effort behavior for static-resolved models.
Useful? React with 👍 / 👎.
|
ClawSweeper PR egg ✨ Hatched: 🥚 common Gilded Crabkin Hatch commandComment Hatchability rules:
Rarity: 🥚 common. What is this egg doing here?
|
e0b6fb0 to
1c07f47
Compare
1c07f47 to
407c0f5
Compare
407c0f5 to
312a123
Compare
ec5cd86 to
a3ccbdd
Compare
8436fc3 to
29c9ce4
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29c9ce435a
ℹ️ 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".
| : options?.skipAgentDiscovery | ||
| ? (resolveStaticCatalogAttempt() ?? (await resolveDynamicAttempt())) |
There was a problem hiding this comment.
Keep OAuth-aware OpenAI resolution before static rows
When skipAgentDiscovery is combined with allowBundledStaticCatalogFallback (the embedded runner now passes both), this resolves any bundled static OpenAI row before resolveDynamicAttempt() can call the provider's resolveDynamicModel with the selected auth profile. OpenAI's dynamic hook uses that auth mode to route OAuth/token profiles for models like gpt-5.4/gpt-5.5 to the Codex transport (openai-chatgpt-responses), while the static catalog row normalizes as the API-key openai-responses transport; for users running the embedded runner with an OpenAI/Codex OAuth profile, this skips the Codex auth bootstrap and attempts the wrong transport. Keep the dynamic attempt ahead of static rows for auth-sensitive providers, or pass equivalent auth-routing context into the static path.
Useful? React with 👍 / 👎.
|
Closing this stale draft PR because it has been open for a long time without merge. I can reopen or resubmit if needed. |
Summary
upstream/mainafter the embedded-agent runner and static-catalog fallback changes.Test Plan
node scripts/run-vitest.mjs src/agents/embedded-agent-runner.e2e.test.ts src/agents/embedded-agent-runner/model.test.tsmodel.test.ts117 passed;embedded-agent-runner.e2e.test.ts14 passed.node scripts/run-vitest.mjs src/agents/bash-tools.exec-foreground-failures.test.tspnpm tsgo:prodtsgo:coreandtsgo:extensions.pnpm check:test-typestsgo:core:testandtsgo:extensions:test.node scripts/check-extension-package-tsc-boundary.mjs --mode=compilegit diff --check upstream/main...HEADReal behavior proof
Behavior addressed: Native Moonshot embedded agent runs can resolve
moonshot/kimi-k2.6from the bundled static manifest catalog before generating OpenClawmodels.jsonor loading provider dynamic hooks, removing the reported model-resolution stall before dispatch.Real environment tested: Local macOS OpenClaw worktree refreshed onto current
upstream/main, using the realsrc/agents/embedded-agent-runner/model.tsruntime entrypoint with isolated temporary OpenClaw home/config directories, an isolated temporary agent directory, and the bundled Moonshot manifest catalog.Exact steps or command run after this patch:
OPENCLAW_HOME=<temp> OPENCLAW_CONFIG_DIR=<temp>/.openclaw OPENCLAW_CONFIG_PATH=<temp>/.openclaw/openclaw.json time -l node --import tsx -e "import fs from 'node:fs/promises'; import path from 'node:path'; import { resolveModelAsync } from './src/agents/embedded-agent-runner/model.ts'; const root = process.env.OPENCLAW_HOME; if (!root) throw new Error('missing OPENCLAW_HOME'); const agentDir = path.join(root, 'agent'); await fs.mkdir(agentDir, { recursive: true }); const started = Date.now(); const result = await resolveModelAsync('moonshot', 'kimi-k2.6', agentDir, { agents: { defaults: { workspace: root } } }, { allowBundledStaticCatalogFallback: true, skipAgentDiscovery: true, workspaceDir: root }); let modelsJsonExists = true; try { await fs.access(path.join(agentDir, 'models.json')); } catch { modelsJsonExists = false; } console.log(JSON.stringify({ resolved: Boolean(result.model), provider: result.model?.provider, id: result.model?.id, api: result.model?.api, baseUrl: result.model?.baseUrl, modelsJsonExists, elapsedMs: Date.now() - started }, null, 2));".Evidence after fix: Copied terminal output from the refreshed runtime command:
{ "resolved": true, "provider": "moonshot", "id": "kimi-k2.6", "api": "openai-completions", "baseUrl": "https://api.moonshot.ai/v1", "modelsJsonExists": false, "elapsedMs": 62116 }The same run also reported
65.17 realseconds and825475072 maximum resident set sizefromtime -l.Observed result after fix: The command exited 0, resolved the Moonshot model from static catalog metadata, did not create
models.json, and used theskipAgentDiscoveryfast path on currentupstream/main.What was not tested: No live Moonshot API call or Discord/TUI end-to-end dispatch was run; the proof isolates the model-resolution path that the issue trace identified, with targeted embedded-runner tests and type/package-boundary checks.
Risk/Notes
allowBundledStaticCatalogFallbackandskipAgentDiscovery; live provider dispatch still depends on operator credentials.