Skip to content

feat: route memory LLM completions through the host model runtime (opt-in transport) - #947

Merged
rwmjhb merged 2 commits into
CortexReach:masterfrom
gorkem2020:feat/runtime-llm-completions
Aug 29, 2026
Merged

feat: route memory LLM completions through the host model runtime (opt-in transport)#947
rwmjhb merged 2 commits into
CortexReach:masterfrom
gorkem2020:feat/runtime-llm-completions

Conversation

@gorkem2020

@gorkem2020 gorkem2020 commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a host-managed completion transport alongside the existing direct OpenAI-compatible client, so the plugin's five completion lanes (extraction, admission-utility, dedup, merge, memory-upgrader) can route through OpenClaw's host-managed runtime LLM catalog instead of always talking to llm.baseURL directly. This is the structural fix requested in issue #901: today the plugin always uses the direct client, which means no provider routing, no app attribution, and ad hoc model-string handling.

What changed

Transport. llm.transport: "direct" | "host", default "direct" (zero behavior change for existing installs), new knob in openclaw.plugin.json's config schema. createHostClient() in src/llm-client.ts sits alongside the existing createApiKeyClient()/createOauthClient(), all three implementing the same LlmClient interface. createLlmClient() picks "host" only when a runtimeLlmComplete function is supplied (feature-detected via api.runtime?.llm?.complete); otherwise it falls back to the direct/oauth transport with one deduped warning per process, not one per client constructed, so a misconfigured or older host never crashes the lane and never spams the log. Model strings pass to the host transport unmodified; the direct client keeps its own openrouter/ prefix-strip normalization, now transport-aware so it never mangles a core-style catalog reference the host transport needs intact. runtime.llm.complete has no AbortSignal parameter, so timeoutMs is enforced with an application-level race instead of true cancellation, and it has no custom-header support, so the existing x-memory-call-label becomes a memory-lancedb-pro:<label> purpose string for equivalent call-site attribution. The host client reuses the same JSON-extraction-and-repair logic as the direct client, so malformed or fenced responses behave identically across transports.

Credential hygiene on fallback. When transport: "host" is configured but the runtime surface turns out to be unavailable, the fallback to the direct client no longer inherits embedding.apiKey/embedding.baseURL as a silent default. On a split-provider setup (embedding via one provider, the LLM lane via host-managed routing) this previously meant the fallback client silently talked to the embedding provider's endpoint with the embedding key instead of failing loudly. It now throws a clear error naming the missing llm.apiKey, caught by the existing surrounding try/catch so it degrades to regex extraction with a warning rather than crashing, and defaults baseURL to OpenRouter's API when llm.apiKey is set but llm.baseURL isn't. The fallback also normalizes a catalog-style model string to the bare provider/model form the direct client needs; an explicitly configured direct transport keeps sending whatever model string it was given, unchanged.

Reasoning effort. Both the host and direct transports now send an explicit reasoning effort on requests: the host transport as a reasoning parameter, the direct transport as reasoning: {effort: ...}. Renamed from llm.reasoningEffort to llm.thinkLevel for consistency with memoryReflection.thinkLevel; llm.reasoningEffort keeps working as a deprecated alias so existing configs don't break. resolveThinkLevel() centralizes the precedence: thinkLevel wins when both are configured (one deduped warning per process), reasoningEffort-only usage keeps working with its own deduped deprecation warning, and blank/whitespace-only values are treated as unset for both keys.

No schema defaults on the effort keys. Neither llm.thinkLevel nor the deprecated llm.reasoningEffort declares a JSON-schema default anymore. A live check found a config that set only llm.reasoningEffort: "low" on disk, but a schema default of "medium" on both keys got materialized into the config object upstream of this plugin on at least one host config-loading path, and once a default is materialized that way it is indistinguishable from a genuine user value by the time it reaches resolveThinkLevel, silently overriding the operator's explicit setting. The code-level "medium" default for the host transport (applied only when neither key is configured) is untouched and still the correct place for that default to live.

Composition with the rest of this wave

Built on plain master before several sibling branches merged, which needed reconciliation once everything came together in the deploy assembly: fix/prompt-architecture's {system, user} prompt split and three-argument completeJson() (the host transport was written with that signature from the start); feat/admission-batch-utility's batch scoring call, which still concatenated system and user into one string for the old two-argument completeJson(); feat/admission-lane-model-affinity's model resolver, which needed a transport-aware skip of the direct-only prefix normalization; and a grounding-v3 interaction where the constructed-grounding short-circuit needed to run before utility scoring on both the single-candidate and batch paths to keep its zero-call guarantee.

A related, pre-existing auto-capture watermark defect (a valid-empty extraction skips the watermark reset a successful extraction applies) was found via the same live probe that surfaced these issues, but is being fixed on its own branch, #948, since it doesn't touch the transport work.

Tests

Red-first throughout. test/llm-host-transport.test.mjs: transport selection, mocked runtimeLlmComplete capturing model and messages, fallback-to-direct with a deduped warning, malformed/throwing host calls, timeout bounding, catalog-model normalization on fallback, and reasoning-effort forwarding (default, explicit override, blank-value fallback). test/admission-control-host-transport.test.mjs and test/batch-admission-host-transport.test.mjs: AdmissionController degrades identically over both transports, including in batch mode. test/admission-model-resolution.test.mjs: transport-aware openrouter/ prefix handling. test/llm-thinklevel-alias.test.mjs: alias precedence and both deduped warnings. test/llm-transport-credential-hygiene.test.mjs: the fallback client never inherits embedding credentials on a split-provider config. test/plugin-manifest-regression.mjs: neither llm.thinkLevel nor llm.reasoningEffort declares a schema default. Existing llm-api-key-client/llm-oauth-client suites pass unchanged, confirming the direct path is unmodified by this PR.

Notes

authProfileId, agentId, and reasoning from the reference runtime-LLM contract aren't all wired through yet; there's no existing config concept in this plugin for auth-profile scoping today, and forwarding reasoning from the plugin is necessary but not sufficient until the host's own runtime-LLM layer also reads and forwards it, since at least one installed core version drops the field before any reasoning-effort default would apply. Can be extended in a follow-up once a host deployment needs it.

Update: remove the llm.reasoningEffort alias entirely (2026-07-17)

llm.reasoningEffort never shipped upstream (zero hits on synced master at the time this was checked), so there is no deprecation constituency to preserve for it. Removed it entirely rather than continuing to carry it as a deprecated alias:

  • resolveThinkLevel no longer reads reasoningEffort or emits either deprecation warning; it is now a plain presence check on llm.thinkLevel.
  • LlmClientConfig drops the reasoningEffort field.
  • The manifest drops the llm.reasoningEffort schema key.
  • llm.thinkLevel is simply the knob's name now.

Test changes: test/llm-thinklevel-alias.test.mjs is renamed to test/llm-thinklevel.test.mjs and stripped down to the single-key behavior (configured / unconfigured / blank). test/llm-host-transport.test.mjs's reasoning-effort tests now configure thinkLevel instead of reasoningEffort. test/plugin-manifest-regression.mjs's assertion that the deprecated key must stay in the schema is inverted to assert it is absent.

Update (2026-07-18)

Design change after operator review: llm.reasoningEffort never shipped in any release, so the deprecation alias is gone entirely. The knob is llm.thinkLevel only; the schema, client, and tests carry no legacy key. (The schema-default override fix from the previous revision is retained.)

…m.transport knob, credential hygiene, 30c-composed llm-client)
@gorkem2020
gorkem2020 force-pushed the feat/runtime-llm-completions branch from f0f9b28 to 90fdf39 Compare August 25, 2026 19:31
@gorkem2020
gorkem2020 marked this pull request as ready for review August 25, 2026 19:31
@gorkem2020

Copy link
Copy Markdown
Contributor Author

Recomposed against current master (post-#988) as a single commit. The llm.transport schema entry now sits under the llm config schema (it was misplaced in an earlier composition), and the transport-aware thinkLevel description rides along. Full suite green locally. Ready for review.

@rwmjhb rwmjhb left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The host-runtime direction is valuable, and the new client-level tests plus current CI are green. On head 90fdf39, however, three production composition/security blockers remain:

  1. Admission strips the catalog provider prefix. resolveAdmissionModel() normalizes the selected model, and admissionClientFor() normalizes it again before constructing the client. With openrouter/anthropic/claude-opus-4-8, extraction keeps the full host catalog reference while admission receives anthropic/claude-opus-4-8. That can bypass the selected catalog provider or fail its allowlist. Please make model normalization transport-aware and add an index.ts composition regression; the current admission test constructs the host client directly and misses this path.

  2. Every host request is an unauthorized model override under the default host policy. createHostClient() always sends model: config.model, and the plugin always populates that field. OpenClaw's runtime checks any supplied model against plugin model-override authority and rejects it unless allowModelOverride/the allowed-model policy permits it. Please omit the model when the operator did not explicitly select one, and document/preflight the policy for explicit catalog models with a policy-enforcing integration test.

  3. Fallback can send an unrelated credential to OpenRouter. When the host surface is unavailable, an absent llm.baseURL is replaced with https://openrouter.ai/api/v1, then combined with any explicit llm.apiKey. An existing OpenAI-compatible provider key can therefore cross into an unrelated endpoint. Please fail closed unless the fallback endpoint is explicit/provider-bound; do not infer a third-party destination for an arbitrary credential.

Also, the advertised OAuth fallback currently checks for apiKey before branching on auth === "oauth", so host-unavailable OAuth configurations cannot reach createOauthClient(). Please cover that while revising fallback behavior.

The full local suite and GitHub checks pass, but their permissive mocks do not exercise these production composition and host-policy cases.

…il-closed fallback

Review round: host transport keeps full catalog model references end to end (admission resolution + index wiring, composition-regression tested); host requests omit the model unless operator-configured; the host->direct fallback refuses to infer a third-party endpoint for llm.apiKey and reaches the OAuth client without one.
@gorkem2020

Copy link
Copy Markdown
Contributor Author

All four findings addressed in bb7f663, thanks for the sharp round:

  1. Model normalization is transport-aware end to end. resolveAdmissionModel() now takes the transport and returns the full catalog reference whole under host (stripping only for direct), and the index.ts client factory does the same. A new index-level composition regression (test/llm-host-transport-composition.test.mjs) registers the plugin with a host runtime stub, drives a real capture, and asserts every host call (extraction and admission utility) carries the full reference.
  2. Host requests now include a model only when it traces to an operator-configured value (llm.model, admissionControl.model, or a lane-affinity memoryReflection.model). Plugin-internal defaults are omitted so the host lane default applies under the default override policy. Covered by the composition test's second case plus the unit cases.
  3. The host-to-direct fallback fails closed without an explicit llm.baseURL: it refuses to infer a third-party endpoint for a configured credential, and the OpenRouter default constant is gone from that path entirely.
  4. Auth is branched before the apiKey requirement, so host-unavailable OAuth configurations now reach the OAuth client (reachability pinned by test).

Full local suite green including the new coverage.

@rwmjhb rwmjhb left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed head bb7f663. All four blockers from the previous review are resolved:

  • admission/extraction preserve full host catalog references through the real index.ts composition path;
  • unconfigured models are omitted so default host policy does not see an override;
  • host-to-direct fallback now fails closed without an explicit llm.baseURL, so arbitrary credentials are not sent to an inferred third-party endpoint;
  • OAuth fallback branches before the API-key requirement and is reachable without an API key.

The focused host/admission/fallback suites pass 37/37, the full suite and required GitHub checks are green, npm run build succeeds, and the generated tree remains clean. Approved.

Non-blocking follow-up: the host timeout currently stops waiting but does not pass an AbortSignal, so the underlying completion may continue consuming quota after timeout.

@rwmjhb
rwmjhb merged commit 160ef42 into CortexReach:master Aug 29, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants