fix(agent): use static catalog for PI model fast path#85696
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed June 12, 2026, 9:23 AM ET / 13:23 UTC. Summary PR surface: Source +10, Tests +46. Total +56 across 4 files. Reproducibility: yes. at source level: the linked issue isolates roughly 34 seconds in model resolution and the branch probe exercises the same runtime entrypoint. A full current-main Discord reproduction and same-environment before/after timing proof are not available. Review metrics: none identified. 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: Keep provider dynamic and auth routing authoritative, then introduce an explicit provider capability or Moonshot-owned fast path that skips expensive discovery only when static metadata is contractually sufficient. Do we have a high-confidence way to reproduce the issue? Yes at source level: the linked issue isolates roughly 34 seconds in model resolution and the branch probe exercises the same runtime entrypoint. A full current-main Discord reproduction and same-environment before/after timing proof are not available. Is this the best way to solve the issue? No. Avoiding broad agent discovery is appropriate, but changing every eligible provider to static-first resolution is broader than the Moonshot bug and violates provider ownership of auth and transport decisions. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 0fc5a57a3440. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +10, Tests +46. Total +56 across 4 files. View PR surface stats
Acceptance criteria:
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 👍 / 👎.
Summary
upstream/mainafter the PI runner rename toembedded-agent-runner.Test Plan
node scripts/run-vitest.mjs src/agents/embedded-agent-runner.e2e.test.ts src/agents/embedded-agent-runner/model.test.tspnpm install: passed 2 Vitest shards;model.test.ts100 passed;embedded-agent-runner.e2e.test.ts13 passed.node_modules/.bin/oxfmt --check src/agents/embedded-agent-runner/run.ts src/agents/embedded-agent-runner/model.ts src/agents/embedded-agent-runner.e2e.test.ts src/agents/embedded-agent-runner/model.test.tsgit diff --check -- src/agents/embedded-agent-runner/run.ts src/agents/embedded-agent-runner/model.ts src/agents/embedded-agent-runner.e2e.test.ts src/agents/embedded-agent-runner/model.test.tspnpm build.Real 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:
/usr/bin/time -l env OPENCLAW_HOME=<temp> OPENCLAW_CONFIG_DIR=<temp>/.openclaw OPENCLAW_CONFIG_PATH=<temp>/.openclaw/openclaw.json node --import tsx -e "import fs from 'node:fs/promises'; import os from 'node:os'; import path from 'node:path'; import { resolveModelAsync } from './src/agents/embedded-agent-runner/model.ts'; const root = await fs.mkdtemp(path.join(os.tmpdir(), 'openclaw-moonshot-fastpath-')); 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": 21974 }The same run also reported
23.08 realseconds and355696640 maximum resident set sizefrom/usr/bin/time -l.Observed result after fix: The command exited 0, resolved the Moonshot model from static catalog metadata, did not create
models.json, and used the renamedskipAgentDiscoveryfast 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 the earlier full local build.
Risk/Notes
node_modulessnapshot was missing the newrastermillpackage; afterpnpm install, the focused tests passed.allowBundledStaticCatalogFallbackandskipAgentDiscovery; live provider dispatch still depends on operator credentials.