fix: align tests and tool defs with SVC_STORAGE delegation#158
fix: align tests and tool defs with SVC_STORAGE delegation#158chitcommit wants to merge 3 commits intomainfrom
Conversation
After 8186f6d delegated evidence tools to ChittyStorage via service binding, three things were left out of sync: - Tests still expected old AI_SEARCH_TOKEN / Vectorize patterns - Tool registry descriptions still referenced AI Search / vector embeddings instead of ChittyStorage document records - credentials.js had a SQL syntax error (unquoted string literal) and a duplicate doc comment opener Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
chittyconnect | a75c6f7 | May 05 2026, 06:17 PM |
There was a problem hiding this comment.
Pull request overview
Updates MCP evidence tool tests and tool metadata to reflect the new ChittyStorage (SVC_STORAGE) delegation model, and fixes a SQL INSERT issue in the credentials PUT route audit logging.
Changes:
- Updated
dispatchToolCallevidence-search/retrieve tests to mock and assertenv.SVC_STORAGE.fetch(...)behavior (instead of prior AI search / Vectorize patterns). - Refreshed MCP tool registry descriptions and input schemas for
chitty_evidence_searchandchitty_evidence_retrieveto match ChittyStorage-backed behavior. - Fixed the credentials PUT route audit INSERT by parameterizing the
typevalue and using a valid timestamp expression; removed a duplicate doc-comment opener.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
tests/mcp/tool-dispatcher.test.js |
Aligns evidence tool tests with SVC_STORAGE binding and ChittyStorage response shapes/formatting. |
src/mcp/tool-registry.js |
Updates tool descriptions and schemas to reflect ChittyStorage delegation and new retrieve inputs. |
src/api/routes/credentials.js |
Fixes SQL INSERT syntax for audit logging in PUT route; cleans up duplicated comment opener. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 924db11c1d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| anyOf: [ | ||
| { required: ["query"] }, | ||
| { required: ["content_hash"] }, | ||
| { required: ["evidence_id"] }, | ||
| ], |
There was a problem hiding this comment.
Enforce a required lookup key in evidence_retrieve schema
This change switches chitty_evidence_retrieve to anyOf-based required fields, but in the ChatGPT/SDK path we register tools from MCP_TOOL_DEFS, and inputSchemaToZodShape only consumes top-level required (not anyOf). That means all three fields become optional during server-side validation, so {} can pass schema checks and only fail later in dispatchToolCall with a runtime error. Please either preserve an enforceable required field here or add anyOf handling in the schema converter so the tool contract is validated before execution.
Useful? React with 👍 / 👎.
|
@claude address feedback and resolve conversations |
|
Claude finished @chitcommit's task in 7m 46s —— View job Addressed PR Feedback
What changed (commit The Codex reviewer correctly identified that
The existing |
…tory
`inputSchemaToZodShape` was ignoring `anyOf: [{required: [key]}, ...]`
schemas, making all fields optional so an empty `{}` would silently pass
Zod validation for `chitty_evidence_retrieve`.
Now detects anyOf-required patterns and returns a `z.object(shape).superRefine(...)`
instead of a plain shape. `server-factory.js` extracts the raw shape for
`McpServer.tool()` registration and runs `safeParse` before dispatching,
returning a structured error if no lookup key is supplied.
Co-authored-by: @chitcommit <chitcommit@users.noreply.github.com>
|
|
@copilot resolve all comments, coversations and conflicts tyo unblcok merge/squash |
The P2 feedback from
All 437 tests pass and all CI checks are green on the latest merge commit (
All review conversations are addressed:
The PR is ready to merge/squash. |
Summary
SVC_STORAGEservice binding mock instead of oldAI_SEARCH_TOKEN/ Vectorize API patternschitty_evidence_searchandchitty_evidence_retrieveto reflect ChittyStorage delegationcredentials.jsPUT route (unquoted string literal in INSERT)credentials.jsFixes 5 test failures introduced by 8186f6d but not caught because the test file wasn't updated in that commit.
Test plan
npx vitest run tests/mcp/tool-dispatcher.test.js— 82/82 pass locally🤖 Generated with Claude Code