refactor: own model discovery by runtime lifecycle#111173
Conversation
openclaw-barnacle
Bot
added
docs
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f598315bc
ℹ️ 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".
| if ((!options?.authStorage || !options?.modelRegistry) && !preparedSnapshot) { | ||
| // Synchronous callers must enter through a lifecycle that already published discovery. | ||
| // Falling back to an empty registry turns a stale/pending generation into a false model miss. | ||
| throw new Error( | ||
| `prepared model runtime is not published for synchronous model resolution (${resolvedAgentDir}); use resolveModelAsync before lifecycle publication`, |
There was a problem hiding this comment.
In a fresh openclaw doctor process, no gateway or agent-run lifecycle has published a prepared snapshot before src/commands/doctor/repair-sequencing.ts:255 invokes collectActiveToolSchemaProjectionWarnings. That collector calls resolveModel without prepared stores, so this new unconditional throw is caught and reported as a model-loading warning for every agent, while the subsequent schema validation loses the model API/compatibility context. Either the doctor flow must publish/load a generation first or this synchronous caller must receive prepared stores; the current unit test mocks resolveModel, so it does not exercise this failure.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
proof: sufficient
|
Codex review: needs maintainer review before merge. Reviewed July 19, 2026, 8:27 AM ET / 12:27 UTC. Summary Reproducibility: not applicable. this is a refactor PR rather than a report of broken existing behavior. The relevant evidence is the supplied after-fix benchmark and focused runtime suites for the lifecycle path. Review metrics: 1 noteworthy metric.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Approve a single public SDK transition only if maintainers accept the deprecated legacy wrapper plus the two lifecycle-owned readers, then land with the documented one-generation ownership model and existing per-agent auth compatibility intact. Do we have a high-confidence way to reproduce the issue? Not applicable: this is a refactor PR rather than a report of broken existing behavior. The relevant evidence is the supplied after-fix benchmark and focused runtime suites for the lifecycle path. Is this the best way to solve the issue? Unclear until maintainers confirm the Plugin SDK contract. The implementation follows the repository’s preferred lifecycle-owned ownership shape, but exposing the new SDK readers and deprecating the existing entry point is a product/API decision. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against e24420bc1c08. Label changesLabel justifications:
Evidence reviewedWhat 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
Review history (9 earlier review cycles; latest 8 shown)
|
67f3fa9 to
7351bd4
Compare
clawsweeper
Bot
added
status: 👀 ready for maintainer look
|
Love seeing this get attention! Thank you so much! |
clawsweeper
Bot
added
rating: 🐚 platinum hermit
7351bd4 to
3a73de1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a73de1671
ℹ️ 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".
| : undefined); | ||
| const workspaceDir = | ||
| options?.workspaceDir ?? preparedSnapshot?.workspaceDir ?? derivedWorkspaceDir; | ||
| const normalizedRef = normalizeProviderModelRef({ provider, modelId, cfg, workspaceDir }); |
There was a problem hiding this comment.
When a config reload completes after the caller captured cfg/agentDir, loadPreparedModelRuntimeSnapshot deliberately rebinds this read to the newly committed owner, but this function adopts only its workspace and continues using the caller's old cfg and resolvedAgentDir for alias normalization, provider config, auth selection, suppression, and runtime hooks. This can combine the new generation's registry and credentials with removed routes, old endpoints, or stale provider policy; use the prepared snapshot's config and agent directory consistently whenever it supplied the stores.
Useful? React with 👍 / 👎.
| const resolved = await (params.modelResolver ?? resolveModelAsync)( | ||
| params.provider, | ||
| params.modelId, | ||
| params.agentDir, | ||
| params.cfg, |
There was a problem hiding this comment.
For TTS, thread titles, progress narration, and the other simple-completion callers, this asynchronous resolver can return a model and stores from a generation committed during a hot reload, but the remainder of this helper still performs route selection, auth lookup, header materialization, and cost/runtime policy with the stale params.cfg and params.agentDir. Even rebinding inside resolveModelAsync cannot fix those downstream reads because its result does not expose the committed context; acquire or return the lifecycle snapshot here so the whole completion uses one generation.
Useful? React with 👍 / 👎.
c23934d to
f1f8431
Compare
|
Merged via squash.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1f8431b5b
ℹ️ 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 preparedRuntimeLease = params.preparedModelRuntime | ||
| ? { snapshot: params.preparedModelRuntime, release: () => {} } | ||
| : await acquireAgentRunPreparedModelRuntime({ | ||
| agentDir: params.agentDir, | ||
| ...(params.agentId ? { agentId: params.agentId } : {}), | ||
| config: params.cfg ?? {}, | ||
| inheritedAuthDir: resolveDefaultAgentDir(params.cfg ?? {}), | ||
| ...(runtimeParams.workspaceDir ? { workspaceDir: runtimeParams.workspaceDir } : {}), |
There was a problem hiding this comment.
When a hot reload commits while runMediaEntry is awaiting image loading/normalization (src/media-understanding/runner.entries.ts:799-828), this lease may rebind to the new config generation, but subsequent resolution still uses the provider, model, auth-profile options, and workspace computed from the old request before the lease. Because the workspace passed here is also treated as explicitly preserved by acquireAgentRunPreparedModelRuntime, image preprocessing can combine the new registry/config with a removed model/profile or the previous agent workspace, causing an avoidable unknown-model/auth failure or routing through stale workspace policy. Acquire the generation before selecting the media entry, or re-select all image routing facts from preparedRuntime.config after acquisition.
Useful? React with 👍 / 👎.
* refactor(agents): prepare model runtime catalogs Build lifecycle-owned model and auth snapshots, carry prepared stores into hot agent paths, and serialize config/auth publication. Credits @zeroaltitude's openclaw#90741 investigation and benchmark approach. * refactor: finish lifecycle-owned model discovery * fix: align prepared model catalog contracts * test: align lifecycle catalog mocks * test: fix prepared catalog type fixtures * refactor: split prepared model runtime ownership * fix: import prepared runtime replacement gate type * test: split media runtime coverage * test: preserve image auth fixture key types * test: isolate lifecycle gate fixtures * chore: keep release changelog owned * refactor: finish prepared model catalog migration * test: keep catalog review fixtures scanner-safe * refactor: preserve lifecycle model runtime ownership * fix: close prepared runtime lifecycle races * fix: preserve compaction workspace fallback * chore: document btw generation rebinding * fix: preserve prepared generation boundaries * fix: keep model-list discovery flag explicit * fix: serialize standalone model runtime activation * refactor: migrate subagent model catalog lookup * refactor: clarify doctor catalog lookup seam * chore: refresh plugin sdk api baseline * test: migrate swarm catalog dependency * refactor(telegram): rename runtime catalog seam * refactor: extract model-aware tool context * test(models): isolate lifecycle catalog fixtures * refactor(agents): avoid btw parameter rebinding * fix(net-policy): align root ipaddr dependency * fix(build): keep net policy dependency bundled * fix(deadcode): document net policy compile dependency
2 participants
Related: #90741
What Problem This Solves
Model catalog consumers repeatedly rebuilt auth stores, provider registries, and projected catalogs on request-time paths. That duplicated filesystem and plugin discovery across agent context, provider auth, session status, replies, cron, doctor, model listing, the TUI, and media tools, while separate caches could expose mismatched auth, registry, and catalog generations.
Why This Change Was Made
This completes the lifecycle-owned catalog program. Gateway/config/auth publication now builds one atomic prepared model runtime generation per configured agent, synchronously stales the previous owner, and lets request paths fork mutable auth/registry state while reusing the generation's catalog snapshot. The context, provider-auth, session-status, auto-reply, cron, doctor, media-understanding, model-list, TUI, subagent spawn, PDF, and image-runtime consumers now use that prepared generation.
The request-time model discovery caches and their obsolete export paths are deleted. The released plugin SDK
loadModelCatalogentry point remains only as a deprecated compatibility wrapper; all internal and bundled callers use the lifecycle-owned API.Thanks to @zeroaltitude for the original performance investigation, measurements, and sponsored direction.
User Impact
Model-aware commands and runtime paths reuse a coherent catalog snapshot instead of paying repeated discovery cost. Auth/config publication swaps the whole generation together, so callers do not observe a new auth store paired with an old registry or catalog. PDF and image sub-runs inherit the parent generation rather than rediscovering models.
Concurrent standalone activations for the same owner are serialized through a per-owner FIFO, which prevents competing direct-command generations from continually staling one another. The late-added subagent structured-output validation path also consumes the prepared catalog rather than restoring request-time discovery.
Evidence
Node 24 benchmark, 20 batches of 25 operations (500 total):
pnpm check:changedgate: green on Blacksmith Testboxtbx_01kxwxdcjwtxe1f81f3da6njqz(run 29682735289) across formatting, SDK API/package/surface/deprecation/boundary checks, all five TypeScript lanes, complete core/extensions/scripts lint, schema/database/media/sidecar/import-cycle/webhook/pairing guardstbx_01kxx0860989xwsmtty7jskzve(run 29684196462) across dependency/export deadcode, all test/script/root type lanes, Plugin SDK surface, and 60 focused lifecycle/BTW testsDesigned Follow-up
The Plugin SDK compatibility facade still owns process-global API-provider publication; each prepared generation snapshots it into its per-registry runtime. Moving publication ownership itself out of process-global state is a designed follow-up, intentionally outside this program.
AI Assistance
Codex assisted with implementation, tests, review, benchmark execution, and PR drafting under maintainer direction.