The openai-api provider may not fake a completion¶
Closes #718. Rig-E runs on this provider (Qwen2.5-32B on vLLM), and it is becoming the operator's task interface (rig-conductor#2104): he reads the reply to decide whether a change was proposed, filed, reviewed or released. Two lines made that reply untrustworthy.
What was wrong¶
| Before | Effect |
|---|---|
for (let turn = 0; turn < 5; turn++) |
llm.maxTurns was dead configuration. rig-e-helmrelease.yaml asks for 15 and got 5; a turn that needed a sixth tool call was cut off silently. |
content?.trim() \|\| 'Done.' |
Two different non-answers — turns exhausted with tool calls pending, and an empty completion — both rendered as the single most misleading word available. |
The second one is the dangerous one: the runtime, not the model, produced a statement of completion that nothing had verified.
What it does now¶
resolveMaxToolTurns(llm)readsllm.maxTurns. Unset keeps the historical5(DEFAULT_MAX_TOOL_TURNS), so no existing character changes behaviour. A value that is not a positive safe integer (0,-3,1.5,'8') is refused with a warning and the default is used —maxTurns: 0would otherwise answer every message without ever calling the model.- On the last allowed turn the provider does not execute the tool calls it cannot read the results of. Nothing can consume them, and running them costs time and can have side effects for an answer that will never exist.
describeReply({ content, toolTurnsExhausted, maxToolTurns })builds the reply:
| State | Reply |
|---|---|
| text, no exhaustion | the text, unchanged |
| text, turns exhausted | the text, then (stopped at the N-tool-turn limit with tool calls still pending — this is not a finished answer) |
| no text, turns exhausted | (no answer: stopped at the N-tool-turn limit …) |
| no text, no exhaustion | (no answer: the model returned an empty reply) |
The string Done. no longer appears in this provider.
Both helpers are pure and exported, so the policy is tested without a network or a model.
Scope¶
Only src/agent/providers/openai-api.js. The other providers still have their own 'Done.'
fallbacks (anthropic-sdk.js:99, claude-cli.js:563, claude-tmux.js:569) — they are not on the
Rig-E path and are left for a separate change. This ships to every agent through the shared
runtime image, so the gitops image pin rewrites apps/dashecorp/rig-e-helmrelease.yaml.
Verification¶
node --test src/agent/providers/openai-api.test.js — 34/34, of which 9 are new: the configured
cap, the default cap, the refusal of a bad value, the exhausted-turn reply, the partial-text reply,
the empty reply, an unchanged real answer, the un-run final tool round, and the two pure helpers.
Full suite 835/835.
Test stability: the hung-completion stub needs a ref'd timer¶
Adding nine tests to openai-api.test.js turned CI red on main (#721, #722) without any of them
failing: # tests 835, # pass 823, # fail 0, # cancelled 12. The twelve cancelled tests are the
pre-existing baseUrlFrom: a hanging completion is aborted by llm.timeoutMs … test and everything
queued behind it in the file, each reported as "Promise resolution is still pending but the event
loop has already resolved".
The cause is in the test's fetch stub, not in the provider. It returned a promise that could only
settle when opts.signal aborted, and the timer behind AbortSignal.timeout() is unref'd — it
does not keep the event loop alive. Verified directly:
const s = AbortSignal.timeout(50);
await new Promise((_, rej) => s.addEventListener('abort', () => rej(s.reason), { once: true }));
// → "Detected unsettled top-level await", process exits 13 before the abort fires
So whether that test settled depended on what else happened to be pending in the process, which is why the same code was green on the PR branch and on the previous main commit. The stub now arms a ref'd 5 s guard timer alongside the abort listener: the loop stays alive until the abort lands, and if the abort genuinely never fires the test fails with its own message instead of cancelling the rest of the file.
Note for anyone reading a red run: cancelled tests report fail 0, so the summary line alone looks
harmless. Read # cancelled.