fix(agent): clarify validateFinalResponse error messages (#45)

[LukasParke]

#45 was not reproducible

Issue #45 reported that getResponse() throws Invalid final response: empty or invalid output for tool-call-only turns. That does not happen on main.

validateFinalResponse (packages/agent/src/lib/model-result.ts) is a pure array-length check — it never inspects item type, so an output containing only a function_call passes cleanly. This is already test-locked at packages/agent/tests/unit/allow-final-response.test.ts:370-390 ("does not trigger when allowFinalResponse is false" asserts the tool-call response is returned unchanged), and this PR adds a direct assertion for it too.

The reporter almost certainly hit a genuinely empty output: []. The error message just didn't say so, and the phrase "invalid output" invited the tool-call theory. This PR fixes the misleading message that caused the misdiagnosis — it changes no logic.

What changed

validateFinalResponse now names the actual defect.

Empty/invalid output — distinguishes the two failure shapes:

• Invalid final response: empty or invalid output — output array is empty (length 0) for response "resp_abc". The model returned no output items. This can happen when the provider returns an empty final turn; see the strictFinalResponse/allowFinalResponse options
• Invalid final response: empty or invalid output — output is not an array (got string) for response "resp_abc"

Missing required fields — lists which field(s) were absent:

• Invalid final response: missing required fields: id
• Invalid final response: missing required fields: output
• Invalid final response: missing required fields: id, output

Both historical prefixes (Invalid final response: empty or invalid output, Invalid final response: missing required fields) are preserved verbatim as prefixes, with detail appended after an em dash, so any consumer matching on them keeps working. A doc comment on the method records that the prefixes are load-bearing.

Consumer audit

Grepped Invalid final response, empty or invalid output, and missing required fields across src, tests, docs, and the Python/Go port paths:

• No src code does message-based control flow (grep '\.message\.includes\|\.message\.match\|\.message ===\|\.message\.startsWith' src/ → zero hits), so no retry logic depends on the exact text.
• packages/agent/CHANGELOG.md:82 quotes the old string historically (PR fix(agent): tolerate empty final response after completed tool rounds #63) — still accurate, since it quotes the prefix.
• tool-terminal-empty-final.test.ts:295,312 used exact-string toThrow(...). Both were substring matches that would have kept passing silently against the new message, so I tightened them to anchored regexes asserting the new specificity — they now verify the id and the empty-array detail rather than just the prefix.
• No README/docs prose quotes these errors.

Tests (red first)

Added packages/agent/tests/unit/final-response-validation.test.ts — 19 cases driving the private validator directly (the not-an-array and missing-field shapes are unreachable through the public callModel path, since the SDK types guarantee an array).

Verified red against unmodified source: 12 failed / 13 passed. Exactly the 10 new-diagnostics assertions failed with Received: "Invalid final response: empty or invalid output", plus the 2 tightened assertions in tool-terminal-empty-final.test.ts. The prefix-stability, single-line, and valid-output guards passed before the change too — confirming they lock existing behavior rather than the new text.

After the fix: 61 files / 763 tests passed, no type errors. build, typecheck, lint all green.

Judgment calls

• allowEmptyOutput still swallows a non-array output. The early return fires before the message is built, so a malformed (non-array) payload is tolerated on the empty-final path — arguably wrong, since that's a broken payload rather than an empty final turn. Left as-is because this PR is diagnostics-only; pinned with an explanatory test so a future logic fix is a deliberate, visible edit.
• No item-type composition summary. The suggested "counts by item type" helper only matters for length-0 arrays, which by definition have no items. Reused the existing describeNonRecord helper for the not-an-array case instead of adding a new one.

Refs #45 — recommend closing the issue as not-reproducible via comment; deliberately not auto-closing from this PR since the reported bug never existed.

🤖 Generated with Claude Code

[perry-the-pr-reviewer]

Perry's Review

Verdict: 💬 Comments / questions

Details

Clean diagnostics-only change. validateFinalResponse now distinguishes empty-array vs. non-array output and lists which required fields are missing, while preserving both historical message prefixes verbatim. No validation logic changed.

What I verified:

• No message-based control flow exists in the agent source — zero hits for message.includes, message.match, or message.startsWith on error strings — so the text change is safe for all consumers.
• describeNonRecord already exists and is used elsewhere; the new call site is consistent.
• Both call sites of validateFinalResponse are unaffected: the strict path still throws on empty/invalid output, and the tolerant path only sets allowEmptyOutput to true when the output is already confirmed to be an array.
• The allowEmptyOutput early-return swallowing non-arrays is pre-existing, acknowledged in the PR description, and pinned with a test — correct call to leave it for a separate logic-change PR.
• The new test file covers every diagnostic branch (empty array, non-array, missing fields, valid outputs). Two existing assertions were tightened from substring matches to anchored regexes — good.
• CI is fully green (lint, typecheck, unit-tests, e2e-tests, structural-gate).

One note: the PR is still a draft. The code looks ready to come out of draft — if you'd like a formal approval after marking it ready, let me know and I'll re-review.