fix(mcp): harden send_feedback against integer-extra drift, error-log echo, and dispatcher shadowing

[gesh]

Problem

While porting the collectFeedback feature (#4870) to posthog-python (PostHog/posthog-python#939), review bots on the Python PR surfaced three hardening gaps that apply to this SDK too. The Python port shipped the fixes; this PR backports them so the two SDKs behave the same.

Changes

1. matchesExtraSchema — a declared integer extra accepted any number, fractional ones included (3.5), because typeof cannot separate int from float. It now requires Number.isInteger per JSON Schema semantics: 3 and 3.0 pass, 3.5 stays out of extras and the captured properties (raw keeps it for the handler).
2. handleFeedback — a thrown onFeedback handler was logged with its full message. A handler can echo the unsanitized report (PII, credentials, log-forging newlines) into its error message, so the log now carries only the error name, matching the report log beside it.
3. prepareToolCall — every call named like the feedback tool was flagged isFeedback, so a dispatcher following the documented if (prepared.isFeedback) ... return flow suppressed a real tool that collides with the name. A host-supplied options.originalTool (already flowing in for model ownership) is stateless proof a real application tool owns the name — the virtual tool never exists in the host's own list — so it now wins and the real tool dispatches normally. Without originalTool the behavior is unchanged.

How did you test this code?

Three new tests in feedback.test.ts (fractional vs whole integer extras across capture + handler surfaces, thrown-handler log carries no error text, originalTool collision disambiguation), plus a logger assertion added to the existing thrown-handler test. Full @posthog/mcp unit suite: 48 files, 876 tests, lint and format clean.

---

Created with PostHog Desktop

[greptile-apps]

Security Review

The error-log mitigation remains incomplete because a thrown Error’s mutable name is logged verbatim, allowing report-derived text to reach host logs if a handler assigns it there.

Prompt To Fix All With AI

### Issue 1
packages/mcp/src/extensions/feedback.ts:355-356
**Exception name remains unsafe**

The handler’s exception name is still interpolated without sanitization. Because `Error.name` is writable, a handler can assign report-derived PII or newline characters to it before throwing, allowing the same log disclosure this change intends to prevent. Log a fixed label or an allowlisted built-in type instead.

**How this was verified:** `onFeedback` receives the agent-controlled report, while `handleFeedback` passes a thrown Error’s mutable `name` directly to the unsanitized host logger.

```suggestion
      logger('Warning: onFeedback handler threw; returning the default acknowledgement')
```

### Issue 2
packages/mcp/src/extensions/posthog-mcp.ts:313
**Wrong absence sentinel**

The new `options.originalTool == null` condition treats `null` as an absence sentinel. This violates the packages/mcp requirement that optional values use `undefined`, so the check must be explicit before merging.

```suggestion
      this.#feedbackOptions !== undefined && name === this.#feedbackToolName && options.originalTool === undefined
```

### Issue 3
.changeset/mcp-feedback-hardening.md:5
**Changeset is too detailed**

This changeset gives a long implementation-level explanation of all three mechanisms. The release guide requires one short, user-facing line that states the fix or feature, so the implementation details must be moved to the PR description before merging.

```suggestion
Harden feedback validation, error logging, and tool-name collision handling.
```

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (1): Last reviewed commit: "fix(mcp): harden send_feedback against i..." | Re-trigger Greptile}

[gesh]

Note

🤖 Automated comment by QA Swarm — not written by a human

Multi-perspective review: router (cheap-first pass) + narrow validation as needed

Verdict: ✅ APPROVE _{(round 4 @ 382e084)}

The custom-dispatcher examples now pass the original application descriptor, and the regression test uses the same contract. The virtual-descriptor concern does not apply because originalTool must come from the application list before PostHog preparation.

Key findings

No open findings.

Convergence

All inline QA threads are resolved. The focused feedback suite and MCP lint pass.

Reviewer summaries

Reviewer	Assessment
🧭 triage	The application-ownership contract is explicit in examples, documentation, and tests.

Previous rounds (3)

round 3 @ 8fd8198 — REQUEST CHANGES: two dispatcher contract findings remained.
round 2 @ fd632f1 — REQUEST CHANGES: unsafe error-name logging and uncertain descriptor ownership.
round 1 @ fd632f1 — APPROVE WITH NITS: update the custom-dispatcher examples.

---

Automated by QA Swarm — not a human review

[gesh]

Note

🤖 Automated comment by QA Swarm — not written by a human

QA Swarm review complete. See inline comments.