Always emit LMNR_HTTP_PORT and align chart-default warm LMNR env#738
Merged
Conversation
The runtime-api warm matcher does strict env equality (clean_env_for_match); LMNR_* keys are not in KEYS_TO_CLEAN, so a missing/extra LMNR key forces a cold start. PR #692 (ec2c290) wrapped LMNR_HTTP_PORT so the request side OMITS it when laminar.httpPort is unset, but the KOTS warm template still always emits all 4 LMNR_* (="" when analytics off) -> only_in_warm=['LMNR_HTTP_PORT'] -> every conversation cold-starts on OHE. Quick, teammate-endorsed fix: make the LMNR_* key set identical on the request side and both warm paths in either toggle state. - _env.yaml: restore LMNR_HTTP_PORT to always-emit (="" when off), matching the other three LMNR_* just above (reverts #692's key omission). - chart-default warm configs (runtime-api + openhands runtime-api override): add LMNR_* (="") so reverting #692 does not re-break the SaaS/chart-default path #692 fixed. - KOTS warm template already always-emits all 4 LMNR_* via shared anchors; kept. Safe because get_agent_server_env auto-forwards all LMNR_* uniformly and the agent only traces when LMNR_PROJECT_API_KEY is set, so LMNR_HTTP_PORT="" is inert (the other 3 already ship "" when off). The cleaner redesign (thread laminar-active consistently through enterprise-server + runtime-api, or harden clean_env_for_match to ignore empty/LMNR_* keys) is a deliberate follow-up. Bump openhands (0.7.64) + runtime-api (0.3.12) chart versions and repin openhands' runtime-api dependency to 0.3.12. Closes #737
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem (#737): every conversation cold-starts on OHE
The runtime-api warm matcher (
warm_runtime_matches->clean_env_for_match) does strict env equality (keys and values), andLMNR_*keys are not inKEYS_TO_CLEAN. So if the request env and a warm pool entry differ on anyLMNR_*key, the warm runtime is rejected and the conversation cold-starts.LMNR_BASE_URL/LMNR_FORCE_HTTP/LMNR_PROJECT_API_KEYare always emitted (=""when off) on both sides and match. But PR #692 (ec2c290) wrappedLMNR_HTTP_PORTincharts/openhands/templates/_env.yamlso the request side OMITS it whenlaminar.httpPortis unset, while the KOTS warm template (replicated/openhands.yaml:356, anchor&lmnr_http_portreused at:625/:913) still always emits it (=""when analytics off):Fix (quick, teammate-endorsed)
Make the
LMNR_*env key set identical on the request side and on both warm paths (KOTS and chart-default), in both toggle states:charts/openhands/templates/_env.yaml: restoreLMNR_HTTP_PORTto always-emit ({{ if and .Values.laminar.enabled .Values.laminar.httpPort }}…{{ else }}""{{ end }}), matching the threeLMNR_*keys directly above it. This reverts APP-2260: omit LMNR_HTTP_PORT env when httpPort is unset #692's key omission.charts/runtime-api/values.yaml(warmRuntimes.configs[].environment) andcharts/openhands/values.yaml(runtime-api.warmRuntimesoverride): add the fullLMNR_*set (=""). Reverting APP-2260: omit LMNR_HTTP_PORT env when httpPort is unset #692 alone would re-break the chart-default/SaaS path (requestLMNR_HTTP_PORT=""but chart-default warm had noLMNR_*keys) — which is what APP-2260: omit LMNR_HTTP_PORT env when httpPort is unset #692 originally papered over. AddingLMNR_*here makes the always-emit revert safe for SaaS.replicated/openhands.yamlalready always-emits all 4LMNR_*via shared anchors (:354-357defined,:623-626+:911-914reused). Verified consistent; left unchanged.Why always-passing
LMNR_HTTP_PORT=""is safeOpenHands/openhands/app_server/sandbox/sandbox_spec_service.py:AUTO_FORWARD_PREFIXES = ('LLM_', 'LMNR_')get_agent_server_env()forwards everyLMNR_*var from the pod env as-is, including empty values — no special-casing.The agent only emits Laminar traces when
LMNR_PROJECT_API_KEYis configured, soLMNR_HTTP_PORT=""is inert when Laminar is off — exactly like the other threeLMNR_*keys, which already ship""in the off state.Render proof (
helm template)Request side (openhands deployment)
LMNR_*key set, both states:LMNR_BASE_URL="",LMNR_FORCE_HTTP="",LMNR_HTTP_PORT="",LMNR_PROJECT_API_KEY=""httpPort=8000): same 4 keys,LMNR_HTTP_PORT="8000",LMNR_FORCE_HTTP="true", base-url/api-key viavalueFromChart-default warm config (runtime-api
warm-runtimes-configmap):{"LMNR_BASE_URL":"","LMNR_FORCE_HTTP":"","LMNR_HTTP_PORT":"","LMNR_PROJECT_API_KEY":"", …}=> Request
LMNR_*key set == chart-default warmLMNR_*key set in both toggle states. KOTS path is equivalent by construction: the anchors render all 4 to""whenanalytics_enabled != 1(matching the off request env) and to real values +laminar.enabled=true/httpPort=8000when on (matching the on request env). KOTS replaces the wholewarmRuntimes.configsarray, so the chart-defaultLMNR_*additions are inert on the KOTS path.Chart versions
openhands0.7.63 -> 0.7.64runtime-api0.3.11 -> 0.3.12, and repinopenhands->runtime-apidependency to 0.3.12 (pervalidate-chart-versions)Follow-up (deliberately deferred)
The cleaner redesign — thread "is Laminar active" consistently through enterprise-server and runtime-api, or harden runtime-api
clean_env_for_matchto ignore empty/LMNR_*keys — is intentionally left as a later follow-up. This PR is the minimal always-pass fix.Closes #737