Skip to content

fix(search_people): accept JSON-stringified network arrays from MCP clients#517

Open
IAmAlexander wants to merge 1 commit into
stickerdaniel:mainfrom
IAmAlexander:fix/coerce-stringified-network-array
Open

fix(search_people): accept JSON-stringified network arrays from MCP clients#517
IAmAlexander wants to merge 1 commit into
stickerdaniel:mainfrom
IAmAlexander:fix/coerce-stringified-network-array

Conversation

@IAmAlexander

Copy link
Copy Markdown

Summary

Some MCP clients serialize array-typed tool arguments whose JSON Schema has no top-level type (the anyOf: [array, null] rendering of list[str] | None) into a JSON string before dispatch. As a result, search_people's network argument arrives as the string '["F"]' instead of the array ["F"], and pydantic rejects it:

Input should be a valid list [type=list_type, input_value='["F"]', input_type=str]

This makes the 1st/2nd/3rd-degree connection filter unusable from affected clients (observed with Claude Desktop / "Cowork"). Clients that send a native array (e.g. the MCP Inspector) are unaffected.

Change

  • network now accepts str | list[str] | None.
  • A string value is normalized to a list (JSON-decoded when possible; otherwise treated as a single token like "F"). Native lists pass through unchanged.
  • The advertised schema becomes anyOf: [string, array<string>, null], so the stringified value also passes validation.

Testing

  • network=["F"] -> ["F"] (unchanged)
  • network='["F"]' (stringified) -> ["F"]
  • network='["F","S"]' -> ["F","S"]
  • network="F" (bare token) -> ["F"]
  • ruff check / ruff format clean.

Co-Authored-By: Oz oz-agent@warp.dev

Some MCP clients serialize array-typed arguments whose JSON Schema has no top-level type (the anyOf[array, null] rendering of list[str] | None) into a JSON string before dispatch, so ["F"] arrives as the string '["F"]' and pydantic rejects it with a list_type error. This makes the connection-degree filter unusable from those clients (e.g. Claude Desktop / Cowork); clients that send a native array work fine.

Accept str | list[str] for network and normalize a string (JSON-decoding when possible, otherwise treating it as a single token) to a list before use. Native arrays are unaffected.

Co-Authored-By: Oz <oz-agent@warp.dev>
@greptile-apps

greptile-apps Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds a pre-flight normalization step to search_people so that MCP clients that JSON-stringify array arguments (e.g. Claude Desktop / Cowork) can successfully pass the network filter. The parameter type is widened to str | list[str] | None and an inline normalizer decodes the string to a list before the tool body runs.

  • The normalization logic covers all documented cases: native lists and None pass through unchanged, a JSON-encoded array string is decoded, and a bare token like "F" is wrapped in a one-element list.
  • Two edge-cases in the json.loads branch ('null'["None"], non-list JSON types) produce unexpected tokens forwarded downstream rather than None or an early error; both would ultimately surface as a FilterValidationError but with a potentially confusing message.
  • No new unit tests accompany the four normalization paths documented in the PR description.

Confidence Score: 4/5

Safe to merge — the normalization is additive and all existing call-sites that pass native lists or None are unaffected.

The change is narrow and well-scoped: it only adds a pre-flight string-normalization step to a single tool, and the happy paths (native list, None, stringified array) all behave correctly. The edge cases around json.loads returning non-list types (null, numbers) produce a slightly unexpected but not broken outcome — downstream validation catches them. The only meaningful gap is the absence of automated tests for the new normalization cases, which leaves the behaviour unguarded against future changes.

linkedin_mcp_server/tools/person.py — the new normalization block and the missing test coverage for it.

Important Files Changed

Filename Overview
linkedin_mcp_server/tools/person.py Adds string-to-list normalization for the network parameter to accommodate MCP clients that JSON-stringify array arguments; logic is correct for all realistic inputs but edge-cases around json.loads returning non-list, non-string JSON types (null, numbers) produce slightly unexpected wrapping instead of None/error. No new unit tests accompany the new normalization paths.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[MCP Client calls search_people] --> B{network type?}
    B -- "list[str]" --> E[Pass through unchanged]
    B -- "None" --> E
    B -- "str" --> C[strip whitespace]
    C -- "empty string" --> D[network = None]
    C -- "non-empty" --> F{json.loads succeeds?}
    F -- "Yes, returns list" --> G[network = parsed list]
    F -- "Yes, returns non-list" --> H["network = [str(parsed)]"]
    F -- "ValueError / JSONDecodeError" --> I["network = [raw_string]"]
    G --> E
    H --> E
    I --> E
    D --> J[extractor.search_people called]
    E --> J
    J --> K{FilterValidationError?}
    K -- "Yes" --> L[Raise ToolError to client]
    K -- "No" --> M[Return search results]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[MCP Client calls search_people] --> B{network type?}
    B -- "list[str]" --> E[Pass through unchanged]
    B -- "None" --> E
    B -- "str" --> C[strip whitespace]
    C -- "empty string" --> D[network = None]
    C -- "non-empty" --> F{json.loads succeeds?}
    F -- "Yes, returns list" --> G[network = parsed list]
    F -- "Yes, returns non-list" --> H["network = [str(parsed)]"]
    F -- "ValueError / JSONDecodeError" --> I["network = [raw_string]"]
    G --> E
    H --> E
    I --> E
    D --> J[extractor.search_people called]
    E --> J
    J --> K{FilterValidationError?}
    K -- "Yes" --> L[Raise ToolError to client]
    K -- "No" --> M[Return search results]
Loading
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
linkedin_mcp_server/tools/person.py:155-159
**JSON null/non-list decoded value produces unexpected network token**

When `json.loads(raw)` succeeds but returns `None` (e.g. input `'null'`) or a number/bool, the `else` branch produces `["None"]`, `["42"]`, etc., which are silently forwarded to the extractor as network filter values. `FilterValidationError` will eventually reject them, but the error message ("None is not a valid network filter") may be confusing. A quick `if parsed is None` guard would set `network = None` in that case and match user intent more naturally.

### Issue 2 of 2
linkedin_mcp_server/tools/person.py:146-159
**No unit tests for the new normalization paths**

The PR description lists four cases that were verified manually (`'["F"]'``["F"]`, bare `"F"``["F"]`, etc.) but no automated test covers them. The existing `test_search_people_with_network_and_company_filters` test only exercises `network=["F"]` (the native-list path). A few parametrized assertions in `tests/test_tools.py` would lock in the behaviour and catch any future regression from ruff auto-fixes or type-annotation changes.

Reviews (1): Last reviewed commit: "fix(search_people): accept JSON-stringif..." | Re-trigger Greptile

Comment on lines +155 to +159
try:
parsed = json.loads(raw)
except ValueError:
parsed = raw
network = parsed if isinstance(parsed, list) else [str(parsed)]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 JSON null/non-list decoded value produces unexpected network token

When json.loads(raw) succeeds but returns None (e.g. input 'null') or a number/bool, the else branch produces ["None"], ["42"], etc., which are silently forwarded to the extractor as network filter values. FilterValidationError will eventually reject them, but the error message ("None is not a valid network filter") may be confusing. A quick if parsed is None guard would set network = None in that case and match user intent more naturally.

Prompt To Fix With AI
This is a comment left during a code review.
Path: linkedin_mcp_server/tools/person.py
Line: 155-159

Comment:
**JSON null/non-list decoded value produces unexpected network token**

When `json.loads(raw)` succeeds but returns `None` (e.g. input `'null'`) or a number/bool, the `else` branch produces `["None"]`, `["42"]`, etc., which are silently forwarded to the extractor as network filter values. `FilterValidationError` will eventually reject them, but the error message ("None is not a valid network filter") may be confusing. A quick `if parsed is None` guard would set `network = None` in that case and match user intent more naturally.

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +146 to +159
# Some MCP clients serialize array arguments whose JSON Schema has no
# top-level "type" (the anyOf[array, null] rendering of `list[str] |
# None`) into a JSON string before dispatch, so ["F"] can arrive as
# the string '["F"]'. Accept and normalize that to a list.
if isinstance(network, str):
raw = network.strip()
if not raw:
network = None
else:
try:
parsed = json.loads(raw)
except ValueError:
parsed = raw
network = parsed if isinstance(parsed, list) else [str(parsed)]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 No unit tests for the new normalization paths

The PR description lists four cases that were verified manually ('["F"]'["F"], bare "F"["F"], etc.) but no automated test covers them. The existing test_search_people_with_network_and_company_filters test only exercises network=["F"] (the native-list path). A few parametrized assertions in tests/test_tools.py would lock in the behaviour and catch any future regression from ruff auto-fixes or type-annotation changes.

Prompt To Fix With AI
This is a comment left during a code review.
Path: linkedin_mcp_server/tools/person.py
Line: 146-159

Comment:
**No unit tests for the new normalization paths**

The PR description lists four cases that were verified manually (`'["F"]'``["F"]`, bare `"F"``["F"]`, etc.) but no automated test covers them. The existing `test_search_people_with_network_and_company_filters` test only exercises `network=["F"]` (the native-list path). A few parametrized assertions in `tests/test_tools.py` would lock in the behaviour and catch any future regression from ruff auto-fixes or type-annotation changes.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

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.

1 participant