Add collect tool type

[mme]

Summary

Adds a collect tool type that lets agents write structured data back to the database, configured entirely in YAML — no code changes needed to define new collect tools.

• Schema validation: Field types (string, number, enum) validated at config load via Zod, with runtime input validation on every tool call
• Generic error handling: Both search and collect tools return generic error messages to agents, preventing internal detail leaks (PG hosts, API keys)
• Config hardening: Tool name uniqueness validation, backwards-compatible type defaulting with deprecation warning, exhaustive tool type dispatch
• First use case: submit-feedback tool for agents to report whether search results were helpful

What changed

Area	Files	What
Types	src/types.ts	CollectToolConfigSchema, discriminated union, field refinements (enum values, non-enum values rejection, min 1 field)
DB	src/db/schema.ts, src/db/queries.ts	collected_data table (JSONB), parameterized insertCollectedData
Implementation	src/mcp/tools/collect.ts, src/mcp/server.ts	yamlSchemaToZod, registerCollectTool, exhaustive switch dispatch
Config	src/config.ts	Tool name uniqueness check, backwards-compat type defaulting, scoped source validation
Error handling	src/mcp/tools/search.ts	Generic error response (was leaking raw error.message)
Logging	src/index.ts	Collect tool call logging with truncated data preview
Config files	mcp-docs.yaml, mcp-docs.example.yaml	submit-feedback tool definition
Tests	src/__tests__/collect*.ts, src/__tests__/tool-config.test.ts	29 tests: schema conversion, config validation, MCP protocol round-trips, DB failure handling
Docs	README.md	Collect tool documentation and examples

Test plan

• npx tsc --noEmit passes
• 29 unit tests via vitest (npm test):
  • yamlSchemaToZod conversion (string, number, enum, required/optional, descriptions, unsupported type rejection)
  • CollectToolConfigSchema and AnyToolConfigSchema validation (empty schema, enum without values, values on non-enum, unknown types)
  • ServerConfigSchema.superRefine for default_limit <= max_limit
  • Full MCP protocol round-trip (list, call, DB write, DB failure with generic error, invalid input rejection)

🤖 Generated with Claude Code

[jpr5]

Code Review — Add Collect Tool Type

Clean, well-scoped PR. The collect tool concept is sound — YAML-defined schemas, Zod validation, parameterized DB writes, good test coverage for a first pass. Found no critical bugs, but several important issues worth fixing before merge.

---

🟡 Important — Should Fix

I1. Error messages from insertCollectedData are exposed verbatim to the MCP agent

src/mcp/tools/collect.ts: when the DB insert fails, the raw error.message is returned in the MCP tool response. PostgreSQL errors routinely include hostnames, ports, usernames, and partial query text. This leaks directly to the LLM agent (and potentially to end users).

Fix: return a generic message in the MCP response, keep the detail in console.error only:

catch (error) {
    const message = error instanceof Error ? error.message : String(error);
    console.error(`[${toolConfig.name}] Error inserting collected data: ${message}`);
    return {
        content: [{ type: "text" as const, text: "Error: Failed to store data. Please try again later." }],
        isError: true,
    };
}

I2. yamlSchemaToZod switch has no default branch

src/mcp/tools/collect.ts: the switch handles 'string', 'number', 'enum' but has no default. If someone adds a new type to the Zod enum in types.ts but forgets to update the switch, fieldSchema is used uninitialized — the server crashes at startup with an incomprehensible TypeError: Cannot read properties of undefined.

Fix: add a default that throws with the field name and type:

default:
    throw new Error(`Unsupported field type "${field.type}" for field "${fieldName}"`);

I3. Empty schema: {} silently accepts and stores arbitrary data

CollectToolConfigSchema allows schema: {}. When registered, this creates a tool with zero input fields that accepts any arguments and writes {} to the database. Almost certainly a misconfiguration.

Fix: add a minimum size on the schema record:

schema: z.record(...).refine(
    s => Object.keys(s).length > 0,
    { message: 'collect tool schema must define at least one field' }
)

The test at tool-config.test.ts that asserts schema: {} is valid should be updated to expect failure.

I4. .default('search') on SearchToolConfigObjectSchema is dead code inside discriminatedUnion

src/types.ts: z.discriminatedUnion inspects the discriminator value before applying defaults. When the type field is absent, Zod can't match any variant — the .default('search') never fires. The actual backward-compat handling is the imperative tool.type = 'search' in config.ts.

Fix: change .default('search') to just z.literal('search') to make the schema honest. The pre-processing in config.ts handles backward compat, and that's fine.

I5. default_limit <= max_limit not enforced by AnyToolConfigSchema

The SearchToolConfigSchema.refine() is only used for type inference, not in the actual parse path. The real enforcement is in ServerConfigSchema.superRefine(). But AnyToolConfigSchema — which looks like the canonical "parse any tool config" entry point — silently allows default_limit > max_limit.

Fix: add a comment on AnyToolConfigSchema noting the cross-field constraint lives in ServerConfigSchema.superRefine, not here. And delete the dead .refine() on SearchToolConfigSchema since it never fires in the parsing pipeline.

I6. MCP test "writes data to the database" is order-dependent

collect-mcp.test.ts: this test asserts insertCollectedData was called, but doesn't invoke the tool itself — it relies on the previous test having run. No vi.clearAllMocks() between tests.

Fix: add beforeEach(() => vi.clearAllMocks()), and either merge the assertion into the "calls the tool" test or make it self-contained.

---

🔵 Suggestions

S1. values accepted on non-enum fields. { type: 'string', values: ['a', 'b'] } parses successfully — values is silently ignored. Consider adding a refine: f => f.type === 'enum' || !f.values.

S2. Config backwards-compat should log a deprecation warning when defaulting type: 'search' so users know to add type: search explicitly.

S3. README says "store it as JSON" but the column is JSONB. Meaningful distinction for PG users.

S4. README description of "Data is written to the collected_data table with the tool name and a timestamp" omits the most important thing stored — the data payload itself.

S5. yamlSchemaToZod JSDoc says "into a Zod object schema" but it returns a shape record, not a z.ZodObject.

S6. DB failure path in registerCollectTool untested. Mock insertCollectedData to reject and verify isError: true.

S7. superRefine for default_limit <= max_limit untested at the ServerConfigSchema level.

---

✅ What's Working Well

• SQL injection safe: insertCollectedData uses parameterized queries ($1, $2)
• Clean discriminated union: AnyToolConfigSchema correctly separates search and collect concerns
• Backwards-compat: config.ts handles existing YAML files without type: field
• Thorough schema validation tests: collect.test.ts covers all field types, required/optional, descriptions, and invalid input rejection
• MCP protocol round-trip test: uses InMemoryTransport to test the full client-server path
• Good first use case: search feedback is genuinely useful and demonstrates the pattern
• Cross-validation scoped correctly: source reference check now only applies to search tools

---

Summary: 6 important issues, 7 suggestions. No critical bugs. The most actionable are I1 (PG error leak to agents) and I2 (switch without default). The schema validation issues (I3-I5) are more about correctness of the validation boundaries.

All 6 requested changes and 7 suggestions addressed

[jpr5]

Round 2 Review — Collect Tool Type

Nice work addressing the Round 1 feedback — 5 of 6 findings fully fixed, 1 documented as intentional.

Fixed from Round 1

Finding	Status
PG error leak to agents	FIXED — generic message returned, detail logged server-side, test verifies no IP leak
switch without default in yamlSchemaToZod	FIXED — throws with field name and type
Empty schema: {} accepted	FIXED — refine requires at least one field
.default('search') dead code in discriminatedUnion	FIXED — z.literal('search'), compat in config.ts with warning
values on non-enum fields	FIXED — refine rejects
default_limit <= max_limit location	Documented — comment explains it lives in ServerConfigSchema.superRefine

Test Gaps Fixed

Gap	Status
DB failure path	FIXED — mocks rejection, verifies sanitized error
superRefine limit validation	FIXED — two tests at ServerConfigSchema level
Order-dependent MCP test	FIXED — vi.clearAllMocks + merged assertions
Empty schema	FIXED — asserts rejection
Backwards-compat config defaulting	Not tested — loadServerConfig() type-injection has no coverage

---

🟡 New Finding — Should Fix

createMcpServer switch has no default case — future tool types silently skipped

src/mcp/server.ts: the switch (tool.type) handles 'collect' and 'search' but has no default. If a new tool type passes Zod validation but isn't wired here, the tool silently doesn't register. Server starts fine, tool never appears, no error.

default:
    throw new Error(`Unknown tool type "${tool.type}" for tool "${tool.name}"`);

---

🔵 Suggestions

• README inconsistency: opening paragraph says "store it as JSON", closing paragraph correctly says "JSONB". Column is JSONB — make consistent.
• registerCollectTool JSDoc: doesn't mention the sanitized error pattern (log detail, return generic message). Worth a one-liner.
• Backwards-compat defaulting untested: the loadServerConfig() type-injection logic has no test. If broken, users without type: in YAML get cryptic Zod errors on startup.

---

✅ Overall

Significant improvement from Round 1. All original important issues addressed. The server.ts default case is the only remaining actionable item — everything else is minor.

[jpr5]

Fixing.

[jpr5]

All Round 2 findings addressed:

• createMcpServer switch now has exhaustive default case (d4d11d2)
• README JSON → JSONB consistency (fd38593)
• registerCollectTool JSDoc documents error handling pattern (b999f1e)
• Backwards-compat config defaulting now tested (6a8bd52)

Build passes, all 31 tests pass. Ready for @AlemTuzlak to review.