test(orchestration): backfill the tool-loop suite and raise its floors

jest.config.js

@@ -55,11 +55,12 @@ export default {
   // because its denominator grew: vertex, mock and global rose, orchestration and its tools fell.
   // That is a finer instrument reading the same suite, which is why these two moved down.
   coverageThreshold: {
-    'src/orchestration/': {statements: 96.96, branches: 92.89, functions: 97.67, lines: 97.1},
-    'src/orchestration/tools/': {statements: 92.77, branches: 92.3, functions: 92.3, lines: 94.28},
+    'src/orchestration/': {statements: 97.02, branches: 93.42, functions: 97.72, lines: 97.14},
+    'src/orchestration/tools/': {statements: 93.02, branches: 92.59, functions: 92.59, lines: 94.36},
     'src/infrastructure/stores/': {statements: 100, branches: 100, functions: 100, lines: 100},
     'src/providers/vertex/': {statements: 94.23, branches: 84.7, functions: 100, lines: 97.72},
     'src/providers/mock/': {statements: 100, branches: 81.25, functions: 100, lines: 100},
-    global: {statements: 91.3, branches: 83.55, functions: 89.79, lines: 91.92},
+    // Measured over the files no path floor above claims, which is what Jest holds `global` to.
+    global: {statements: 95.05, branches: 90.7, functions: 93.75, lines: 95.74},
   },
 };

specs/hal-engine-architecture/spec.md

@@ -9,9 +9,9 @@
 
 ## The Big Picture
 
 The system has three layers: a frontend client, a Node.js backend (hal-engine), and a pluggable AI provider. They communicate over WebSocket for real-time streaming.
 
 Only the middle layer is this repository. The `Client` group below, the `applyUpsert`/`applyDelta`/`applyCommit` functions in Entry Streaming Protocol, the reconnection and unmount behaviour under Connection Lifecycle, and the spinner logic in Tool Execution Loop all describe a browser client. They are here because the protocol only makes sense as a pair, not because any of it ships from here.
 
 ```mermaid
 graph LR
@@ -75,9 +75,9 @@
 
 ### SessionStore
 
 Controls where session data is stored: the default `InMemorySessionStore` keeps sessions in a `Map` ([validated by: creates and retrieves a session](../../src/infrastructure/stores/inMemorySessionStore.test.ts#L13)). Implement this interface to persist sessions in Redis, a database, or any other backing store. The store is generic -- pass your own session type extending `BaseSession`.
 
 <!-- doc-block: src/types/sessionStore.ts#SessionStore -->
 ```typescript
 interface SessionStore<T extends BaseSession = ChatSession> {
   create(sessionId: string, userId: string | number, options?: SessionCreateOptions): Awaitable<T>;
@@ -118,7 +118,7 @@
 
 ### WsAuthenticator
 
 Authenticates WebSocket connections during the handshake. It receives the whole Node `IncomingMessage`, not a pre-extracted token, so it can read credentials from wherever the client put them. Return the authenticated user to accept the connection, or `null` to reject it with HTTP 401 -- a rejection is a returned `null`, not a thrown error.
 
 <!-- doc-block: src/types/auth.ts#WsAuthenticator -->
 ```typescript
@@ -387,14 +387,22 @@
 
 ### Whether a round continues
 
-- A round continues only when the model stopped for `tool_use`, named at least one tool, and a `ToolRegistry` is configured ([validated by: continues when the model requested a tool and a registry can run it](../../src/orchestration/orchestratorHelpers.test.ts#L11)).
-- With no registry configured the loop stops, whatever the model asked for, because nothing could execute the call ([validated by: stops when no registry is configured, whatever the model asked for](../../src/orchestration/orchestratorHelpers.test.ts#L15)).
-- A `tool_use` stop naming no tool stops the loop, rather than re-querying with nothing to add ([validated by: stops when the model named no tool](../../src/orchestration/orchestratorHelpers.test.ts#L19)).
-- Any other stop reason ends the loop even when tool calls are pending ([validated by: stops when the model finished for a reason other than tool use](../../src/orchestration/orchestratorHelpers.test.ts#L23)).
+- A round continues only when the model stopped for `tool_use`, named at least one tool, and a `ToolRegistry` is configured ([validated by: continues when the model requested a tool and a registry can run it](../../src/orchestration/orchestratorHelpers.test.ts#L18)).
+- With no registry configured the loop stops, whatever the model asked for, because nothing could execute the call ([validated by: stops when no registry is configured, whatever the model asked for](../../src/orchestration/orchestratorHelpers.test.ts#L22)).
+- A `tool_use` stop naming no tool stops the loop, rather than re-querying with nothing to add ([validated by: stops when the model named no tool](../../src/orchestration/orchestratorHelpers.test.ts#L26)).
+- Any other stop reason ends the loop even when tool calls are pending ([validated by: stops when the model finished for a reason other than tool use](../../src/orchestration/orchestratorHelpers.test.ts#L30)).
+- The gate is the whole truth table of its three conditions: of the eight combinations of stop reason, pending calls and registry, only `tool_use` with a pending call and a registry continues ([validated by: continues for tool_use with a pending call and a registry alone, and stops for the other seven rows](../../src/orchestration/orchestratorHelpers.test.ts#L61)).
+- Seen from outside, a scripted `tool_use` turn with no registry ends after one round: one provider call, no executor, and the next scripted round never streams ([validated by: ends a scripted tool_use turn after one round with no registry: one provider call, the tool_use and stop chunks alone](../../src/toolLoop.test.ts#L172)).
+- A round that stops with `end_turn` despite carrying a tool call ends the turn with zero executions ([validated by: ends after one round with zero executions when the stop reason is end_turn despite a tool call: the second scripted round never streams](../../src/orchestration/chatOrchestrator.test.ts#L884)).
+- A stream that ends without a `stop` chunk leaves the stop reason empty, so the turn ends after that round with zero executions and no second provider call ([validated by: ends after one round with zero executions when the stream ends without a stop chunk: the second scripted round never streams](../../src/orchestration/chatOrchestrator.test.ts#L888)).
+- The helpers the round is built from are pure. `collectText` joins the text chunks of a stream and ignores every other type ([validated by: joins the text chunks of a stream in order and ignores every other chunk type](../../src/orchestration/orchestratorHelpers.test.ts#L87)).
+- `collectToolCalls` appends a `tool_use` chunk's call to the pending list and ignores the rest ([validated by: appends the call of a tool_use chunk to the pending list and leaves it alone for text and stop chunks](../../src/orchestration/orchestratorHelpers.test.ts#L95)).
+- `extractStopReason` reads a stop chunk and answers `null` otherwise ([validated by: reads the reason off a stop chunk and answers null for a text chunk](../../src/orchestration/orchestratorHelpers.test.ts#L108)).
+- `extractUsage` reads a stop chunk's usage and answers `undefined` for a stop without one or any other chunk ([validated by: reads the usage off a stop chunk that reports one, and answers undefined for a stop without usage and for a text chunk](../../src/orchestration/orchestratorHelpers.test.ts#L118)).
 
 ### Across rounds
 
-- A tool call's `signature`, an opaque token its provider needs back, is copied onto its `tool_use` block for the next round, and a call without one gets no `signature` key ([validated by: puts sig-1 on the signed call's tool_use block and no signature key on the unsigned one](../../src/orchestration/orchestratorHelpers.test.ts#L29)).
+- A tool call's `signature`, an opaque token its provider needs back, is copied onto its `tool_use` block for the next round, and a call without one gets no `signature` key ([validated by: puts sig-1 on the signed call's tool_use block and no signature key on the unsigned one](../../src/orchestration/orchestratorHelpers.test.ts#L36)).
 - A call to a tool name the model invented is answered rather than raised, with a result naming it and the registered tools, and the turn goes on ([validated by: answers a call to a tool name the model invented, and the turn goes on to its answer](../../src/orchestration/chatOrchestrator.test.ts#L757)).
 - So is a call whose executor throws: it gets a result saying the call failed, its message reaches only the log, as [tool-executor-throw](../hal-engine-tool-executor-throw/spec.md) specifies, and the turn goes on ([validated by: answers a call whose executor threw, and the turn goes on to its answer with afterModelResponse and no onError](../../src/orchestration/chatOrchestrator.test.ts#L770)).
 - A tool signals a failed call by returning `isError: true` beside its `result`, and the engine marks the failures it substitutes the same way, so the provider is told the call failed instead of leaving the model to infer it from prose; [the tool responses spec](../hal-engine-tool-responses/spec.md) gives each provider's encoding ([validated by: carries isError true from an executor's {result: 'lookup failed', isError: true} onto the tool_result the provider's next call gets](../../src/orchestration/toolResultError.test.ts#L62)).
@@ -405,6 +413,14 @@
 - The usage `afterModelResponse` receives is `totalUsage`: the sum of what every provider call in the turn reported, so a turn that ran tools counts every round rather than the last one alone ([validated by: sums the usage of every round, not only the last](../../src/orchestration/chatOrchestrator.test.ts#L616)).
 - A round that reports no usage drops out of the sum rather than voiding it: the rounds that did report are still counted, and a turn where nothing reported reports nothing ([validated by: counts every reporting round even when a round between them reports none](../../src/orchestration/chatOrchestrator.test.ts#L638)).
 
+### A round with more than one call
+
+- A round carrying two calls runs both executors, and the next round's request carries one assistant message with two `tool_use` items and one user message with two `tool_result` items whose ids match, in the order the model asked ([validated by: runs both executors of a two-call round and sends the next round one assistant message with two tool_use items and one user message with two tool_result items whose ids match, in script order](../../src/toolLoop.test.ts#L63)).
+- When one of two executors suppresses the answer, exactly one `suppress_output` chunk follows the round's `tool_result` chunk ([validated by: yields exactly one suppress_output chunk, after the tool_result chunk, when one of two executors suppresses](../../src/toolLoop.test.ts#L91)).
+- Two `entry_upsert` messages from one round take sequential indices from the session's length at call time, and both entries are readable from the session afterwards ([validated by: gives two upserted entries sequential indices from the session length at call time, and both are readable from the session afterwards](../../src/toolLoop.test.ts#L113)).
+- A round whose executors return no client messages yields no `tool_result` chunk at all ([validated by: yields no tool_result chunk for a round whose executors return no client messages](../../src/toolLoop.test.ts#L148)).
+- The pending calls are a fresh list each round, so a call from one round never reaches the next round's request ([validated by: carries no pending call from one round into the next: three rounds leave two assistant messages with one tool_use item each, and each executor ran once](../../src/toolLoop.test.ts#L159)).
+
 The frontend shows a spinner on the last tool entry while the stream is still processing (`isProcessing` is `true`). The `stream_end` message clears the processing state, which hides the spinner. This works correctly even when tool suppression prevents assistant entries from reaching the frontend. See [tool-responses.md](../hal-engine-tool-responses/spec.md) for details on client messages and suppression.
 
 ## Thinking Tag Parsing

src/orchestration/chatOrchestrator.test.ts

@@ -862,3 +862,30 @@
 
   return {outcome: await outcomeOf(chunksFrom(stream)), callOrder};
 }
+
+// A scripted first round, then a second round that must never stream; the registry holds the tool TOOL_CALL names and records every run.
+const gatedTurn = async (firstRound: MessageChunk[]) => {
+  const ran: string[] = [];
+  const provider = scriptedProvider([firstRound, [{type: 'text', text: 'never'}, STOP_DONE]]);
+  const registry = new ToolRegistry();
+  registry.register({name: 'lookup', description: 'looks things up', inputSchema: {type: 'object'}}, async () => {
+    ran.push('lookup');
+    return {result: 'found'};
+  });
+
+  const chunks = await chunksFrom(
+    createChatOrchestrator(provider, promptBuilder, registry).processMessageStream(createSession('ask'))
+  );
+
+  return {ran, types: chunks.map(c => c.type)};
+};
+
+describe('ChatOrchestrator tool loop gating, observed from outside', () => {
+  it('ends after one round with zero executions when the stop reason is end_turn despite a tool call: the second scripted round never streams', async () => {
+    expect(await gatedTurn([TOOL_CALL, STOP_DONE])).toEqual({ran: [], types: ['tool_use', 'stop']});
+  });
+
+  it('ends after one round with zero executions when the stream ends without a stop chunk: the second scripted round never streams', async () => {
+    expect(await gatedTurn([TOOL_CALL])).toEqual({ran: [], types: ['tool_use']});
+  });
+});

src/orchestration/orchestratorHelpers.test.ts

@@ -1,6 +1,13 @@
-import {buildToolCallMessages, shouldContinueToolLoop} from './orchestratorHelpers.js';
+import {
+  buildToolCallMessages,
+  collectText,
+  collectToolCalls,
+  extractStopReason,
+  extractUsage,
+  shouldContinueToolLoop,
+} from './orchestratorHelpers.js';
 import type {ToolRegistry} from './tools/registry.js';
-import type {ToolCall} from '../types/ai.js';
+import type {MessageChunk, ToolCall} from '../types/ai.js';
 
 // All three conditions matter; dropping the registry check spins the tool loop.
 
@@ -41,3 +48,80 @@ describe('buildToolCallMessages', () => {
     });
   });
 });
+
+const STOP_FOR_TOOL: MessageChunk = {type: 'stop', stopReason: 'tool_use'};
+const STOP_DONE: MessageChunk = {type: 'stop', stopReason: 'end_turn'};
+const TEXT: MessageChunk = {type: 'text', text: 'word '};
+
+const streamOf = async function* (chunks: MessageChunk[]): AsyncGenerator<MessageChunk> {
+  yield* chunks;
+};
+
+describe('shouldContinueToolLoop truth table', () => {
+  it('continues for tool_use with a pending call and a registry alone, and stops for the other seven rows', () => {
+    const rows = [
+      ['tool_use', A_TOOL, REGISTRY],
+      ['tool_use', A_TOOL, undefined],
+      ['tool_use', [], REGISTRY],
+      ['tool_use', [], undefined],
+      ['end_turn', A_TOOL, REGISTRY],
+      ['end_turn', A_TOOL, undefined],
+      ['end_turn', [], REGISTRY],
+      ['end_turn', [], undefined],
+    ] as const;
+
+    expect(rows.map(([reason, calls, registry]) => shouldContinueToolLoop(reason, [...calls], registry))).toEqual([
+      true,
+      false,
+      false,
+      false,
+      false,
+      false,
+      false,
+      false,
+    ]);
+  });
+});
+
+describe('collectText', () => {
+  it('joins the text chunks of a stream in order and ignores every other chunk type', async () => {
+    const tool: MessageChunk = {type: 'tool_use', toolCall: {id: 't1', name: 'lookup', input: {}}};
+
+    expect(await collectText(streamOf([TEXT, tool, {type: 'text', text: 'by word'}, STOP_DONE]))).toBe('word by word');
+  });
+});
+
+describe('collectToolCalls', () => {
+  it('appends the call of a tool_use chunk to the pending list and leaves it alone for text and stop chunks', () => {
+    const pending: ToolCall[] = [];
+    const call: ToolCall = {id: 't2', name: 'weather', input: {city: 'here'}};
+
+    collectToolCalls(TEXT, pending);
+    collectToolCalls({type: 'tool_use', toolCall: call}, pending);
+    collectToolCalls(STOP_FOR_TOOL, pending);
+
+    expect(pending).toEqual([call]);
+  });
+});
+
+describe('extractStopReason', () => {
+  it('reads the reason off a stop chunk and answers null for a text chunk', () => {
+    expect([extractStopReason(STOP_FOR_TOOL), extractStopReason(STOP_DONE), extractStopReason(TEXT)]).toEqual([
+      'tool_use',
+      'end_turn',
+      null,
+    ]);
+  });
+});
+
+describe('extractUsage', () => {
+  it('reads the usage off a stop chunk that reports one, and answers undefined for a stop without usage and for a text chunk', () => {
+    const usage = {inputTokens: 3, outputTokens: 4, totalTokens: 7};
+
+    expect([extractUsage({...STOP_DONE, usage}), extractUsage(STOP_DONE), extractUsage(TEXT)]).toEqual([
+      usage,
+      undefined,
+      undefined,
+    ]);
+  });
+});