perf(agent): end the agent loop once every required tool has been called

[Decipher]

Refs #70.

The problem

runAgentLoop returns only when the model answers with no tool calls. But several agents
deliver their entire output through a tool:

• a review sub agent, through submit_review_handoff
• the review writing agent, through post_pull_request_comment and evaluate_pull_request
• the pull request reply, inline reply, issue open and issue reply agents, each through
  their own required tool

Once that tool has run, there is nothing left for the model to say. The loop nonetheless
sends one more request, purely to receive a message with no tool calls so it can return.

That request is the most expensive one the agent makes. The transcript is never longer than
it is at the end of a run, and all of it is re-sent to be told there is nothing more to do.
A review is a plan agent, N sub agents and a writing agent, so the pipeline pays for it
N+2 times, in latency and in input token alike.

The change

A new loop option, endWhenRequiredToolsCalled. When it is set, the loop returns as soon
as every tool in requiredToolList has been called at least once, immediately after that
step's tool results are appended.

It is off by default, so nothing changes for an agent that is meant to keep working
after calling a required tool. It is turned on for the review sub agents and the writing
agent, where the required tool is the agent's output by definition.

The reply and issue agents would benefit identically and the one line is the same, but I
have left them alone so this can be judged on the review pipeline first.

finalMessage becomes the assistant content from the step that called the tool, which may
be null. Nothing in the codebase reads finalMessage outside the loop's own tests, and
both review services discard it, so this is not a behavioural change for any caller.

Why not just stop requiring the extra turn for everyone

Because the loop cannot tell, in general, whether a required tool is an agent's output or
one step of its work. The existing mechanism only enforces that the tool is called before
finishing. An opt-in keeps that distinction with the caller, who knows which it is.

Tests

Five new cases in loop.test.ts, all counting requests rather than asserting on messages,
since the saved request is the point:

• stops after the required tool runs, one request not two
• still asks for another turn when the option is off
• waits for every required tool, not just the first
• does not end early when only an optional tool was called
• leaves an agent with no required tool unaffected

Three of the five fail without the loop change; the other two assert unchanged behaviour
and pass either way, which is intended.

Full suite is 71 pass, 0 fail. tsc --noEmit on apps/api is clean. Prettier is clean on
every file this touches except writing.service.ts, which already fails on main for an
unrelated import wrap; I have left that alone rather than add unrelated churn to this
diff.

What this does not fix

The two larger causes in #70 are untouched: maxSteps defaulting to 100 per agent with
no production caller passing it, and sub agent fan-out width, which on my instance tracks
duration more closely than diff size does. This change removes a fixed N+2 requests from
every review; it does not bound the variable part.

[proval-0e6d07]

Thanks for this — a tight, well-scoped change that removes a wasted confirmation turn from the two review agents without touching the other required-tool call sites.

Overview

This PR adds an opt-in endWhenRequiredToolsCalled option to runAgentLoop and enables it for the review sub agent and writing agent. Once every required tool has executed, the loop returns immediately instead of re-sending the full transcript for a turn that only confirms completion — the largest request of each agent's run. The option is off by default and documented at apps/api/src/agent/llm/loop.ts:68-76; only writing.service.ts:119 and sub.service.ts:58 turn it on, so the issue/PR reply agents keep their existing behavior. The early-exit condition is guarded by toolCallCount, which only increments after execute() resolves (loop.ts:326), so a failed required tool cannot end the loop early, and the sub agent's post-loop handoff check (sub.service.ts:63-70) still fails loudly if the invariant ever breaks.

Good points

• 🟢 The core test asserts expect(sent).toHaveLength(1) with a sender that would fail on a second call (apps/api/src/agent/loop.test.ts → apps/api/src/agent/llm/loop.test.ts:212), directly pinning the one-request cost saving, alongside option-off, multi-required-tool, optional-only, and no-required-tool boundary cases.
• 🟢 The option is off by default with a doc comment explaining both the saving and the risk of cutting short an agent meant to keep working (apps/api/src/agent/llm/loop.ts:68-76); verified the other four requiredToolList call sites leave it unset.
• 🟢 toolCallCount increments only after await tool.execute() resolves (apps/api/src/agent/llm/loop.ts:326), so the early exit at loop.ts:358-362 can never fire on a thrown required tool — the existing "Required tool not called" nudge keeps the loop alive for a retry.
• 🟢 The sub call site pairs the fast exit with an explicit post-condition: the handoff is re-checked after the loop with stepCount/toolCallCount diagnostics (apps/api/src/agent/pull-request/review/sub.service.ts:63-70).

Additional Notes

• apps/api/src/agent/llm/loop.test.ts:184, the new describe block never exercises a required tool whose execute() rejects — worth one test where it fails first and succeeds on the second attempt, asserting the loop didn't end after the failing turn, so a future refactor that moves the count increment before execute() can't slip through green.

[seoes]

@Decipher

Great. I was aware of the fact that the agent loop always produces the final answer but didn't make any update for it

It'll make much faster agent loop

thank you for made this change!