Skip to content

fix(mcp): accept embedded custom fields without a type - #74

Merged
stephendolan merged 2 commits into
mainfrom
claude/confident-dijkstra-b7q1z4
Oct 1, 2026
Merged

fix(mcp): accept embedded custom fields without a type#74
stephendolan merged 2 commits into
mainfrom
claude/confident-dijkstra-b7q1z4

Conversation

@stephendolan

@stephendolan stephendolan commented Sep 30, 2026

Copy link
Copy Markdown
Owner

Fixes #56.

Problem

Help Scout's conversation list/search embed returns custom fields as { id, name, value, text }, with no type. customFieldSchema in src/mcp/server.ts required type. Because conversationSchema uses that schema, every conversation-returning MCP tool (search_conversations, search_by_customer, list_conversations, get_conversation) failed output validation whenever a conversation had custom fields set:

MCP error -32602: Output validation error: ... conversations[0].customFields[0].type
  Required (expected string, received undefined)

.passthrough() (from #50) doesn't help here, because the field is missing rather than extra.

Change

  • customFieldSchema: type is now optional, and the embed's text label is declared as optional.
  • CustomField type updated to match.
  • Regression tests call all four tools through the in-memory MCP client, using a conversation shaped like the real embed. All four fail on main and pass with this change.

Verification

  • bun run typecheck, bun run lint, bun run format:check: clean
  • bun run test: 102 passed

🤖 Generated with Claude Code

https://claude.ai/code/session_01NkDvdNG1AWbYCDzZ6XbXEz

Generated by Claude Code

claude and others added 2 commits September 30, 2026 23:11
@claude
fix(mcp): accept embedded custom fields without a type (#56)
5a9952f 
Help Scout's conversation list/search embed returns custom fields as
{ id, name, value, text } with no `type`. customFieldSchema required
`type`, so every conversation-returning MCP tool (search_conversations,
search_by_customer, list_conversations, get_conversation) failed output
validation whenever a conversation had custom fields set.

Make `type` optional, declare the embed's `text` label, and mirror both
in the CustomField type. Adds regression coverage for all four tools.

Fixes #56

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NkDvdNG1AWbYCDzZ6XbXEz
@stephendolan
test(mcp): preserve typed and embedded custom fields together
328fd74
@stephendolan
stephendolan marked this pull request as ready for review October 1, 2026 13:08
@stephendolan
stephendolan merged commit 99f3902 into main Oct 1, 2026
1 check passed
CLBarajas added a commit to CLBarajas/helpscout-cli that referenced this pull request Oct 20, 2026
@CLBarajas
Merge upstream v2.19.0 + stephendolan#71/stephendolan#74 (assign adop…
5ca0601 
…ted; stephendolan#56 fix adopted, fork copy dropped)

Takes all four commits in main..upstream/main (99f3902): v2.19.0 (stephendolan#68, stephendolan#69)
plus two unreleased ones after the tag (stephendolan#71, stephendolan#74).

ADOPTED
- stephendolan#74 (99f3902) customFields[].type optional + `text` declared. This closes
  our upstream issue stephendolan#56. Semantically identical to the fork's 20d18fe;
  customFieldSchema is now byte-identical to upstream's, and the fork's
  6-line block comment (outside the conflict hunk) is removed with it.
  The fork's 20d18fe regression test is DROPPED: upstream's four end-to-end
  tests (search_conversations, search_by_customer, list_conversations,
  get_conversation) cover it, and its fixture held real customer data.
- stephendolan#68 (b2e1236) `conversations assign` / MCP assign_conversation /
  updateConversationAssignee / stricter parseIdArg. Upstream reimplemented
  the fork's update --assignee; both send the same PATCH /assignTo op.
  Kept the fork's update / update_conversation beside it (broader: status,
  subject, customer, mailbox in one call).
- stephendolan#71 (1fe99c8) @napi-rs/keyring 1.3.0 -> 2.0.0, vitest 4.1.10 -> 5.0.0.
  keyring 2.0.0 verified against the real macOS Keychain: reads all four
  existing helpscout-cli items byte-identically to 1.3.0, and a throwaway
  item round-tripped write/overwrite/delete across both versions (same item,
  no duplicate). bun.lock taken as auto-merged (upstream's lock + the fork's
  mime-types lines); --frozen-lockfile passes and install leaves it unchanged.
- stephendolan#69 (0a8eb20) version 2.19.0.

SILENT BREAKAGES FIXED (no conflict markers; the build accepts both)
- updateConversationAssignee passed ONE patch object to the fork's
  array-typed updateConversation (8937fd4). tsup/esbuild builds it; at
  runtime `for...of` throws, so every assign would have failed. Upstream's
  CLI/MCP tests mock the method; only typecheck and upstream's two
  api-client assign tests see it. Wrapped in [op]. Same drift the v2.15.0
  merge (45df349) hand-fixed for updateConversationStatus.
- conversations.ts imported HelpScoutCliError twice (tsc TS2300).
- stephendolan#68's file-level beforeAll raw-connected the singleton MCP server, so the
  fork's hazard-3 dialect lock ("labels every emitted schema 2020-12") threw
  "Already connected to a transport". Confirmed failing before the fix. The
  shared connection now goes through connectMcpServer (the production path
  that applies retargetSchemaDialect) and the dialect test reuses it, so a
  future raw connect fails that test instead of bypassing it.

Conflict resolution (6 hunks / 3 files)
- mcp/server.ts: customFieldSchema -> upstream's; test exports unioned
  (fork's four + getMcpServerForTesting).
- mcp/server.test.ts: fork stephendolan#56 test dropped; subject-less, tool-parity and
  scope-floor tests kept; upstream's four assign_conversation tests and the
  stephendolan#74 embed describe kept; subject-less comment repointed.
- lib/api-client.test.ts: both hunks unioned (all fork tests + upstream's
  assign / unassign PATCH-shape tests).

Gates: baseline at 6f4dafb 163/163 (vitest 4.1.10); merged 181/181 under
vitest 5.0.0 (-1 dropped, +19 upstream). typecheck 0, lint 0, format at the
pre-existing 12. Beyond the gates: literal registerTool 110 -> 111 (only
+assign_conversation, zero lost, no duplicates); built binary loads (no
Commander duplicate), reports 2.19.0; assign's three invalid-argument paths
fail before any API call. Real-stdio tools/list: 173 tools, all 25
outputSchemas labelled 2020-12 and compiling under Ajv2020; live read-only
get_conversation (#188422, subject-less, type-less customFields),
search_conversations and list_draft_replies validate under Ajv2020. Keychain
auth with env fallbacks unset passes on keyring 2.0.0.
CLBarajas added a commit to CLBarajas/helpscout-cli that referenced this pull request Oct 20, 2026
@CLBarajas
docs: reconcile ledgers with the v2.19.0 merge
8718f06 
UPSTREAM_SYNC: current state written after the fast-forward (main at the
merge 5ca0601 + this commit; 0 behind / 88 ahead of upstream 99f3902; 6
ahead of origin/main; rollback tag/branch pre-upstream-2.19.0 at 6f4dafb,
plus the bun install step a rollback also needs). Merge-history row for
v2.19.0 + stephendolan#71/stephendolan#74; pending back to empty; the keyring 2.0.0 Keychain
verification; the attribution finding the open-items table referred to but
never recorded; unmerged upstream branches re-derived (all superseded).

Hazards: hazard 1 recurred as signature drift (an upstream caller of the
fork's array-typed updateConversation; same drift 45df349 hand-fixed at
v2.15.0), plus a duplicate import. New hazard 4: a test-harness double
connect that broke the hazard-3 dialect lock. Hazard 3 phase corrected per
modelcontextprotocol/typescript-sdk#2721: calls fail client-side, the tools
stay listed. The v2.18.3 tool-count method corrected (107 -> 110 by literal
name) and the extractor pinned. Gates: bun run test is vitest, bun test is
not, and upstream CI runs bun test; format baseline is now 12.

UPSTREAM_ISSUES_PLAN: stephendolan#56 fixed upstream by stephendolan#74 and merged, our test
dropped (its fixture held customer data); stephendolan#73 traced and not ours; the
feature candidates annotated with what upstream shipped on its own.
API_COVERAGE: assign/unassign, 173 tools / 25 outputSchemas, the draft
status-guard and attribution text corrected, a duplicate System Users
section removed. V3_ROADMAP: issue status and counts.

The same hazard-3 phase wording is fixed in the retargetSchemaDialect
docblock and the dialect-test comment (comments only). CLAUDE.md's test
command is bun run test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

MCP output validation failure: customFields[].type is required but list/search embed omits it

2 participants

@stephendolan @claude