fix(dsh-runtime): emit a failed result frame when model selection derivation throws - #7082
Conversation
Non-browser HTTP clients can forge Sec-Fetch-Site trivially, so a no-Origin request carrying 'Sec-Fetch-Site: same-origin' must not widen the host check to the full OD_ALLOWED_ORIGINS allow-list on the non-loopback path. Only a loopback/private-LAN host (or an explicitly configured IP-literal origin) may omit the Origin header; reverse-proxy deployments with a hostname in OD_ALLOWED_ORIGINS must send Origin or use bearer auth instead. Fixes nexu-io#7041
…ivation throws Model selection (provider/id → resolved selection) was derived before the execute error path, so a throw there (e.g. default-model service unavailable, or a rejected host-supplied value) rejected the task promise with no result frame at all; serve then exited the process silently and the host waited on a result that never arrived. Derive inside a guarded block that emits an explicit failed frame, and make the serve loop convert any stray rejection into a failed frame as defense in depth so the 'exactly one result per execute' protocol contract holds.
lefarcen
added
size/M
|
Queued for QA validation — this touches runtime behavior, so we'll want a manual QA pass before merge. Also, it looks like you already have #7081 open with the same One small PR-body follow-up: could you add a short |
nettee
left a comment
nettee
left a comment
There was a problem hiding this comment.
Reviewed the DSH runtime error framing and daemon origin hardening. Focused validation passed: pnpm guard, workspace pnpm typecheck, DSH runtime tests/typecheck, and daemon origin tests/typecheck. I found one protocol-blocking cleanup path and one cancellation edge case; both are detailed inline below.
| // unhandled rejection (which would make the process exit silently and | ||
| // leave the host waiting on a result that never arrives). | ||
| void task | ||
| .catch((error: unknown) => { |
There was a problem hiding this comment.
Blocking: this fallback unconditionally synthesizes another terminal result whenever execute rejects, but execute can reject after it has already written its terminal result: the outer try writes the result before the finally, where disposeEvent() and cleanup still run. A disposer/cleanup error therefore produces two result frames (the original terminal frame plus this fallback). The DSH protocol requires exactly one terminal result, and the daemon marks any frame after finished as fatal, so a successful or failed run can be turned into a fatal protocol error. Track a terminal-frame-written state shared with execute, or make cleanup errors non-rejecting, and emit this fallback only when no result was written; add a serve-level cleanup-error test that asserts one result.
| ? { ...baseSelection, reasoningEffort: ReasoningEffortId(request.reasoning_effort) } | ||
| : baseSelection; | ||
| } catch (error: unknown) { | ||
| writeFrame(output, { |
There was a problem hiding this comment.
Non-blocking: this new selection-error branch bypasses cancellation semantics. serve aborts taskAbort as soon as it handles a cancel, and the existing execute catches explicitly check signal.aborted before emitting a terminal frame. If currentSelection() throws while that signal is already aborted (for example, a settings/default-model service is unavailable during an immediate cancel), this branch emits DSH_PROFILE_INVALID_MODEL_SELECTION instead of cancelled, so a user cancellation is reported as a failed run. Check signal.aborted before writing this failed frame and add a serve-level execute-plus-cancel regression test.
4 participants
Why
Found while auditing
@open-design/dsh-runtimev0.1.0 (the DeepSeek Harness runtime plugin this app ships). The pain: model selection derivation (currentSelection()/ReasoningEffortId()/SessionId()) runs outsideexecute's error path. A throw there — e.g. the default-model service is unavailable, or a host-supplied value is rejected — rejects the task promise with no result frame at all;servethen exits the process silently (exit 0) and the host waits on a response that never comes. I reproduced the silent exit against a realdsh --profile open-design --stdiosession before fixing: only thereadyframe was emitted, then the process vanished.What users will see
Nothing in the app UI. Protocol robustness: an
executewhose model selection cannot be derived now receives an explicitfailedresult frame (DSH_PROFILE_INVALID_MODEL_SELECTION) instead of a silently exiting stdio process, so Open Design (and any other host) always gets exactly one result per execute — the protocol contract in the plugin README.Surface area
error.codevalue (DSH_PROFILE_INVALID_MODEL_SELECTION) may appear on already-failed result frames.Screenshots
N/A.
Bug fix verification
packages/dsh-runtime/tests/protocol.test.ts→ "emits a failed result frame when model selection derivation throws (no silent exit)" (mocksagentDefaultModel.currentSelection()to throw and asserts the execute resolves with exactly onefailedresult frame).mainand green on this branch? Yes. Onmainthe test fails (the throw rejects the task promise before any frame is written); on this branch all 13 tests pass.mainbehavior viadsh --profile open-design --stdiobefore the fix: ready frame only, then silent exit. After the fix the same host-supplied invalid value returns an explicitfailedresult frame.pnpm --filter @open-design/dsh-runtime typecheckpasses.