Skip to content

test(questionTool): add parser coverage and reject unsupported fields#1631

Merged
zerob13 merged 1 commit into
ThinkInAIXYZ:devfrom
lil-goat:chore/add-tests-questiontool
May 16, 2026
Merged

test(questionTool): add parser coverage and reject unsupported fields#1631
zerob13 merged 1 commit into
ThinkInAIXYZ:devfrom
lil-goat:chore/add-tests-questiontool

Conversation

@lil-goat
Copy link
Copy Markdown
Contributor

@lil-goat lil-goat commented May 15, 2026

Summary

  • add direct unit coverage for parseQuestionToolArgs
  • verify normalization, default values, and jsonrepair recovery behavior
  • reject unsupported top-level fields like allowOther and questions by making the top-level schema strict

Why

src/main/lib/agentRuntime/questionTool.ts did not have dedicated unit coverage, and its Zod schema
silently accepted unsupported top-level keys when the required fields were otherwise valid. That
made the documented tool contract looser in practice than the prompt guidance claims.

Verification

  • pnpm exec vitest run test/main/lib/agentRuntime/questionTool.test.ts
  • pnpm exec vitest run test/main/presenter/toolPresenter/toolPresenter.test.ts -t "describes the question schema and returns actionable validation errors"
  • pnpm run lint
  • pnpm run typecheck
  • pnpm run build

Notes

  • Non-UI change; no screenshots needed.
  • Verification was run with the bundled Codex Node runtime v24.14.0, so npm scripts emit an
    engine warning because the repo asks for >=24.14.1 <25.

Summary by CodeRabbit

  • Improvements

    • Question tool validation now strictly enforces expected fields, rejecting payloads with unsupported or unknown fields for improved data integrity.
  • Tests

    • Added comprehensive test suite for question tool argument parsing, covering JSON recovery, normalization, and validation scenarios.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 15, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4490415a-adda-4e04-82fb-412c4ec13d3f

📥 Commits

Reviewing files that changed from the base of the PR and between 5727aff and f23c953.

📒 Files selected for processing (2)
  • src/main/lib/agentRuntime/questionTool.ts
  • test/main/lib/agentRuntime/questionTool.test.ts

📝 Walkthrough

Walkthrough

This PR tightens question tool argument validation by adding Zod .strict() mode to reject unknown properties, then provides comprehensive test coverage verifying normalization, JSON repair, error messaging, and rejection of unsupported fields.

Changes

Question Tool Strict Validation

Layer / File(s) Summary
Schema validation tightening
src/main/lib/agentRuntime/questionTool.ts
questionToolSchema now applies Zod .strict() to disallow unknown top-level properties during validation of question tool arguments.
Test coverage for strict validation
test/main/lib/agentRuntime/questionTool.test.ts
Comprehensive parseQuestionToolArgs tests verify normalization and defaults, recoverable JSON repair, contract-hint error messaging on invalid JSON, and rejection of unsupported top-level fields.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 Strict schemas keep the fields so neat,
No stragglers slipping through the seat.
Tests verify each JSON dance—
Repair and reject, give validation a chance! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main changes: adding test coverage for the question tool parser and enforcing stricter validation by rejecting unsupported fields.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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

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

@zerob13
Copy link
Copy Markdown
Collaborator

zerob13 commented May 16, 2026

LGTM

@zerob13 zerob13 merged commit 9a415a0 into ThinkInAIXYZ:dev May 16, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants