fix(ask_followup_question): report non-array follow_up as a type error

[nh2]

Related GitHub Issue

Closes: #511.

Description

A native tool call for ask_followup_question with a present-but-non-array follow_up (e.g. a keyed object produced by incremental JSON parsing) was forwarded unchecked and then rejected by the tool via recordMissingParamError, which always reports the misleading message

Missing value for required parameter follow_up

The value was the wrong type, not missing, which sent the model into a retry loop with the same payload.

Fix:

AskFollowupQuestionTool.execute now distinguishes the cases: null/undefined follow_up still reports a missing parameter, while a present-but-non-array value reports a clear type/shape error (surfaced to both the user via say(error) and the model via toolError) instructing it to retry with a JSON array.

NativeToolCallParser keeps forwarding the raw follow_up value on the finalize path (with an explanatory comment) rather than guarding with Array.isArray, so the tool can emit the precise error instead of the call being silently dropped to null by the generic parser guard.

Adds test coverage for string and keyed-object non-array follow_up values, plus a parser finalize test confirming the value is forwarded rather than dropped.

Assisted-By: Claude Opus 4.8 in Zoo Code.

Test Procedure

I let Zoo add some tests which look sane and pass, and I'm test-triving the built .vsix currently.

Pre-Submission Checklist

• Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
• Scope: My changes are focused on the linked issue (one major feature/fix per PR).
• Self-Review: I have performed a thorough self-review of my code.
• Testing: New and/or updated tests have been added to cover my changes (if applicable).
• Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
• Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

Documentation Updates

Additional Notes

Get in Touch

I'm not really using Discord but I read mentions here on Github.

Summary by CodeRabbit

• Bug Fixes
  • Improved validation and error handling for the ask follow-up question tool. Invalid parameter types now trigger precise error messages that clearly specify the expected array format, enabling users to quickly identify and resolve configuration mistakes.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 871f148e-3045-4698-af0e-34774b6a960a

📥 Commits

Reviewing files that changed from the base of the PR and between 1b5830c and a7cb46b.

📒 Files selected for processing (3)

• src/core/assistant-message/NativeToolCallParser.ts
• src/core/tools/AskFollowupQuestionTool.ts
• src/core/tools/__tests__/askFollowupQuestionTool.spec.ts

---

📝 Walkthrough

Walkthrough

AskFollowupQuestionTool.execute() now distinguishes between a missing follow_up (undefined/null → missing-parameter error) and a present but non-array follow_up (→ new type-validation error: "must be an array"). NativeToolCallParser is annotated to preserve non-array follow_up values so the tool receives them intact. Tests are updated and extended accordingly.

Changes

follow_up Type Validation Fix

Layer / File(s)	Summary
AskFollowupQuestionTool: split missing vs. wrong-type validation src/core/tools/AskFollowupQuestionTool.ts	Adds inline comment documenting the runtime non-array possibility, then splits the prior combined guard into two branches: undefined/null triggers recordMissingParamError; present-but-non-array triggers a new recordValidationError path that emits formatResponse.toolError with a "must be an array" message and returns early.
NativeToolCallParser: annotate non-array follow_up preservation src/core/assistant-message/NativeToolCallParser.ts	Adds a comment in the ask_followup_question switch case of parseToolCall explaining that nativeArgs must retain a non-array follow_up so the tool can emit the precise type error rather than a generic missing/parse failure.
Tests: non-array follow_up type errors and parser preservation src/core/tools/__tests__/askFollowupQuestionTool.spec.ts	Replaces the old single non-array test with two unit tests (string and keyed-object) asserting the "must be an array" error path, no sayAndCreateMissingParamError call, and no task.ask invocation. Adds an integration test that NativeToolCallParser.finalizeStreamingToolCall forwards a keyed-object follow_up to the tool unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• Zoo-Code-Org/Zoo-Code#212: Directly rewrites and extends unit tests for AskFollowupQuestionTool, including explicit follow_up validation cases that this PR reshapes.

Suggested reviewers

• taltas
• hannesrudolph
• navedmerchant

Poem

🐇 Hop, hop — the follow_up was lost,
A looping model at such a cost!
Now arrays are checked with a knowing eye,
Wrong types get errors, not a silent goodbye.
"Must be an array!" the rabbit declares,
No more infinite loops — the warren cheers! 🎉

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: fixing the ask_followup_question tool to report non-array follow_up values as type errors instead of missing parameter errors.
Description check	✅ Passed	The description comprehensively covers the issue, the fix, test coverage, and includes all required sections from the template with appropriate checkmarks.
Linked Issues check	✅ Passed	The code changes directly address issue #511 by distinguishing between missing and invalid-type follow_up parameters, providing clear error messages to prevent retry loops [#511].
Out of Scope Changes check	✅ Passed	All changes are focused on the ask_followup_question tool error handling and related parser/test updates, staying within the scope of fixing the infinite retry loop issue.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[nh2]

Hmm, after some time I got another such failure again even with my patch in:

Roo tried to use ask_followup_question without value for required parameter 'follow_up'.

Not sure yet how that can be.

[nh2]

I found that with my new problem, the model generates <parameter name="follow_up">.

That is even though src/core/prompts/sections/tool-use.ts says explicitly:

Use the provider-native tool-calling mechanism. Do not include XML markup

which seems to suggest a training bias to emit XML (I'm using Claude Opus 4.8 here).

[edelauna]

Thanks for addressing this. I'm wondering if we should file a ticket for the xml findings you mention, and at the very least emit a retry signal with updated instructions?