Select action response-header timeouts from the manifest - #93
Open
CryptoJones wants to merge 3 commits into
Open
CryptoJones wants to merge 3 commits into
CryptoJones wants to merge 3 commits into
Conversation
executor.go defined two HTTP clients — a 30s response-header bound for everything and a 10m one reserved for Ollama's cold load — and actions.go chose between them with a literal engine == "ollama" && action == "run_model" comparison. Every other engine got 30 seconds for every action, no manifest field could change it, and the shipped lmstudio.json chat action (and any vLLM manifest) inherited the same ceiling: a long prefill against a large model failed with "net/http: timeout awaiting response headers", which reads like a broken engine rather than a client-side limit. Make the budget a property of the action instead: an optional per-action timeout_s in the manifest, defaulting to the existing 30s when unset (or equal to it). The executor resolves one cached HTTP client per distinct declared value and reuses the shared 30s client otherwise. Ollama's cold run_model moves its 600s into manifests/ollama.json, and the engine-name comparison and ollamaLoadClient field are deleted. Validation rejects a negative timeout_s and a timeout_s on a non-http action so it can never be a silent no-op. The total call stays bounded by the executor's action timeout regardless. Tests: the name-based selection test is replaced by one that pins the new behavior (a declared timeout_s is honored for any engine name; an undeclared action on an engine named ollama gets the ordinary budget), a client-caching unit test, validation accept/reject cases, and a pin on the bundled ollama run_model declaration so it cannot silently regress. Fixes NVIDIA#25. Signed-off-by: Aaron K. Clark <cryptojones@owasp.org>
The new optional per-action timeout_s manifest field is an additive feature visible over the engine:action surface, so MINOR for the component (0.17.4 -> 0.18.0). Product and installer follow the MINOR component bump per VERSIONING.md (0.91.7 -> 0.92.0). Signed-off-by: Aaron K. Clark <cryptojones@owasp.org>
Issue NVIDIA#25 was reported against the lmstudio chat action, but the mechanism commit only turned the new timeout_s knob on for Ollama's run_model. Without an opt-in, lmstudio chat still gets the 30s response-header default and the reported timeout is unchanged. Declare 600s (matching Ollama's cold run_model) so a cold model load or long prefill no longer cuts off the first byte; the total call stays bounded by the executor's action timeout. Pin it with a regression test against the bundled manifest and document the field on the engine:action row. Signed-off-by: Aaron K. Clark <cryptojones@owasp.org>
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.
Description
Closes #25.
nvpair-engine-managerdefined two HTTP clients — a 30s response-header bound for everything and a 10m one reserved for Ollama's cold load — andactions.gochose between them with a literalengine == "ollama" && action == "run_model"comparison. Every other engine got 30 seconds for every action, no manifest field could change it, and the shippedlmstudio.jsonchat action (and any vLLM manifest) inherited the same ceiling: a long prefill against a large model failed withnet/http: timeout awaiting response headers, which reads like a broken engine rather than a client-side limit.This makes the response-header budget a per-action manifest field:
timeout_son http actions (seconds). Validation rejects negative values and placement on non-http actions at manifest load.timeout_sgets exactly the same 30s budget as before; third-party manifests are unaffected.ollamaLoadClientfield and the name comparison are gone; the client is selected by declared value and cached per distinct value (all default-budget actions share one client).run_modeldeclares 600s in the bundled manifest — behavior preserved, now data-driven.chatdeclares 600s in the bundled manifest — the action [Bug]: Action HTTP response-header timeout is selected by a hardcoded engine name, so only Ollama gets the long client #25 was reported against now gets a budget that fits a cold model load or long prefill before the first byte. The total call stays bounded by the executor's per-action timeout (default 30 min), so a declared header budget can never unbound a call.Scope
In:
registry.go(field + validation);executor.go(client selection/caching); both bundled manifests; the READMEengine:actionrow; tests;versions.json.Out (deliberately): JSON-RPC surface changes —
timeout_sis manifest data, not a wire field, so no broker relay, desktop bridge, orservice-contractschanges; per-engine defaults in code (the manifest is the single source of truth); an upper cap ontimeout_s(every action call is already wrapped in the 30-min context deadline).Validation
Go 1.26.0 on Linux (x86_64). Desktop tree untouched, so the npm gates were not run.
gofmt -l/go vet ./...innvpair-engine-managerlmstudio.json,ollama.json)go test ./...inservices/nvpair-engine-manager(incl. 6 new tests)-raceNew tests:
TestEngineHTTPClientBoundsResponseHeaders,TestActionTimeoutSFollowsManifestNotEngineName,TestBundledLMStudioChatDeclaresResponseHeaderBudget,TestActionClientCachesPerDeclaredTimeout,TestValidateAcceptsActionTimeoutS,TestBundledOllamaRunModelTimeout. The twoTestBundled*tests load the realgo:embedmanifests and pin that both declaring actions keep their long budgets, so a future manifest edit that silently reverts this fix fails CI.Independent review. The diff was reviewed by a panel of fourteen independently trained models (Anthropic ×2, OpenAI, Google, Z.AI, NVIDIA Nemotron ×2, Mistral, DeepSeek, xAI, Moonshot AI, Nous Research, Amazon, and a local Qwen), one at a time, each blind to the others. Tally: 2 approve, 9 approve-with-nits, 3 request-changes — all three request-changes lanes on the same finding. Every finding was verified against the code before being acted on: (a) confirmed real and fixed in this branch — lmstudio's
chataction never declaredtimeout_s, so the reported action would still have received the 30s default; it now declares 600s with a regression pin; (b) refuted — no upper cap is needed because every action call is wrapped in the 30-min context deadline; (c) confirmed minor and fixed — the README now documents the field; (d) refuted — the executor already guards non-positive/default values before building a client.Risk
chat's response-header budget rises from 30s to 600s for the first byte only; body streaming is unchanged and the total call remains capped by the per-action timeout. Every other action keeps its existing behavior (30s default). Ollamarun_model: same 600s, now declared in data instead of hardcoded.product/installerto 0.92.0 inversions.json; if it lands first this branch needs a one-line rebase (the values match).Versions
nvpair-engine-manager0.17.4 → 0.18.0 (MINOR: additive optional manifest field on theengine:actionsurface). Perservices/VERSIONING.md, a MINOR component bump impliesproduct/installer0.91.7 → 0.92.0.AI disclosure
This change was written with AI assistance and reviewed by a panel of AI models, under the direction of the human author, who tested it on his own hardware and takes responsibility for it.
qwen3.8-27b) running as a Hermes agent, operated by Aaron K. Clark (CryptoJones), who set the design decisions (per-action manifest field over an engine-name special case; 600s matching Ollama's existing cold-load budget; MINOR version bump) and ran all validation on his own hardware.fable), Claude Opus (opus) — via theclaudeCLIgpt-5.6-solgemini-3.1-pro-highz-ai/glm-5.3-flash(via OpenRouter)nvidia/nemotron-3-ultra-550b-a55b,nvidia/nemotron-3-super-120b-a12b(via OpenRouter, free tier)mistralai/mistral-large-2512(via OpenRouter)deepseek/deepseek-v4-flash(via OpenRouter)x-ai/grok-4.6(via OpenRouter)moonshotai/kimi-k2.6(via OpenRouter)nousresearch/hermes-4-405b(via OpenRouter)amazon/nova-pro-v1(via OpenRouter)qwen3.8-27bon the review hostTranscripts of every lane's answer are retained by the author and can be shared with maintainers on request.
Checklist
git commit -s), certifying the Developer Certificate of Origin.services/versions.json, and described user-visible changes above so they reach the release notes.Proudly Made in Nebraska. Go Big Red! 🌽 https://xkcd.com/2347/
🤖 Authored in Hermes (
qwen3.8-27b) under the direction of Aaron K. Clark