Harden object/array parameters against LLM stringification (v2.8.0)

[devondragon]

Summary

• Add parseObjectParam() and parseArrayParam() utilities for defensive JSON parsing when LLM transports serialize structured parameters as strings
• Apply to autoScheduled (PR Harden object/array parameters against LLM stringification (v2.8.0) #89 original fix), frequency (recurring tasks), status array (task list), and value array (custom field multiSelect)
• Bump version to 2.8.0 across package.json, mcp-server.ts, worker.ts
• Update CHANGELOG with 2.8.0 entry
• Fix stale README tool descriptions (motion_search and motion_workspaces)

Test plan

• 452 unit tests pass (npm test)
• 26 integration tests pass against live Motion API (npm run test:integration)
• npm run build succeeds
• New tests for parseObjectParam, parseArrayParam, stringified frequency, and stringified frequency error case

Closes #88

[claude]

Code Review

This is a clean, focused PR that addresses a real-world LLM behavior issue. Good problem/solution framing and solid test coverage. A few minor observations below.

---

src/utils/parameterUtils.ts

The implementation is correct and well-placed. Checking startsWith('{') before attempting JSON.parse is a nice early-exit optimization that avoids parsing strings that cannot possibly be objects.

Minor: No test for '{}' (empty JSON object)
'{}' would be parsed as {} and returned directly. This enables auto-scheduling without a schedule name (matching the behavior of value === true), which will then fail schedule validation downstream. The behavior is consistent with the existing design, but an explicit test would document this edge case clearly.

Minor: Pre-existing dead code (not introduced by this PR)
After the string block exits (lines 104-112), there are checks for value === 'true' and value === 'false' as strings. Since the string block handles both 'true' and 'false' and returns early, those branches are unreachable for string inputs. Worth cleaning up in a follow-up, but out of scope here.

Observation: No validation of parsed object shape
'{"unknownKey":"value"}' would be returned as { unknownKey: 'value' } and passed to the Motion API. This is consistent with how native objects are handled today (pass-through), so it is by design — just worth noting that the function is purely structural and relies on downstream validation.

---

src/tools/ToolDefinitions.ts

The updated description is clearer and the explicit instruction ("When the user specifies a start date, you MUST use the object form.") is a good prompt engineering choice.

Minor: Stale example date
The example uses "startDate": "2025-03-05", which is now in the past (today is 2026-03-02). Using a clearly future date or a placeholder like "YYYY-MM-DD" would avoid confusion for users reading the schema.

---

tests/parameterUtils.spec.ts

The 5 new tests cover the key paths well:

• JSON object with schedule only
• JSON object with schedule + startDate
• JSON object with schedule + deadlineType
• Invalid JSON starting with { falls back to schedule name
• JSON array string (does not start with {) falls through to schedule name

Suggestion: Add a test for '{}' to document the empty-object / "enable without schedule" edge case explicitly.

The test for '["Work Hours"]' is a useful regression guard even though it covers a pre-existing code path.

---

Summary

Area	Status
Logic correctness	Good
Error handling	Good
Security	Good (JSON.parse is safe here, no code execution)
Test coverage	Good, one missing edge case
Description improvement	Good, with minor stale date nit

Overall this is ready to merge. The missing '{}' test and stale example date are both minor nits that can be addressed now or in a follow-up.

[Copilot]

Pull request overview

This PR improves robustness of the Motion MCP server’s autoScheduled parameter handling when invoked by LLMs, addressing cases where models provide JSON-stringified objects instead of native objects and clarifying tool guidance for startDate.

Changes:

• Add defensive parsing in parseAutoScheduledParam() to detect and parse JSON-stringified object inputs.
• Update the autoScheduled tool description to more explicitly document string vs object forms and when to use each.
• Add unit tests covering JSON-stringified inputs and graceful fallback behavior for invalid JSON.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
src/utils/parameterUtils.ts	Adds JSON parsing for string inputs that look like serialized objects before falling back to schedule-name shorthand.
src/tools/ToolDefinitions.ts	Expands autoScheduled description with explicit examples and guidance for startDate.
tests/parameterUtils.spec.ts	Adds 5 tests validating JSON-string handling and fallback behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The new JSON.parse branch returns whatever object is parsed without checking for prototype-pollution keys (e.g., "proto", "constructor", "prototype"). Downstream code spreads autoScheduled (e.g., in TaskHandler.validateAndNormalizeAutoScheduling), so a maliciously crafted JSON string could tamper with object prototypes or cause surprising behavior. Consider only accepting plain objects and rejecting/sanitizing dangerous keys before returning the parsed value (e.g., validate prototype === Object.prototype and strip reserved keys).

[Copilot]

Now that JSON-stringified objects are supported, the JSDoc “Supported formats” list above is incomplete (it still implies any non-empty string becomes {schedule: string}). Please update the documentation to mention the JSON-stringified-object behavior so future changes/tests don’t drift from the actual parsing rules.

[Copilot]

The autoScheduled schema now shows a JSON-form object example, but the string shorthand’s description still uses a non-JSON example ({schedule: 'name'}). This inconsistency can confuse LLM callers; consider updating the shorthand description/example to valid JSON formatting (quoted key/value) to align with the main description.

Suggested change

	{ type: "string", description: "Schedule name (shorthand for {schedule: 'name'})" }
	{ type: "string", description: "Schedule name (shorthand for { \"schedule\": \"name\" })" }

[claude]

Code Review — v2.8.0: Harden object/array parameters against LLM stringification

This is a well-scoped, defensive PR addressing a real operational problem (LLM transports serializing structured parameters as JSON strings). The core utilities are clean and the test coverage is solid. A few observations across the full diff:

---

src/utils/parameterUtils.ts — New utilities

parseObjectParam and parseArrayParam are clean, symmetric utilities with correct semantics:

• Pass-through for already-correct native types
• JSON.parse with a fast-exit prefix check (startsWith('{') / startsWith('['))
• Silent failure returning undefined for unparseable inputs
• Array check (!Array.isArray) in parseObjectParam correctly rejects [...]-parsed results

The barrel file (src/utils/index.ts) already uses export * from './parameterUtils', so no explicit re-export was needed.

Pre-existing dead code (not introduced here): In parseAutoScheduledParam, the check value === 'true' on line 105 is unreachable for string inputs — the string block on lines 79–102 already handles 'true' and returns early. Worth a follow-up cleanup.

---

src/handlers/CustomFieldHandler.ts — Duplicated parsing logic

The multiSelect defensive parsing block is copy-pasted between add_to_project (lines 99–113) and add_to_task (lines 129–143). These are identical except for variable name (value vs taskValue). If a third call site appears this duplication could cause inconsistent behavior — consider extracting a private helper.

---

src/handlers/TaskHandler.ts — Redundant outer check

The outer guard before calling parseArrayParam duplicates what parseArrayParam already does internally. Since parseArrayParam returns native arrays as-is, calling it unconditionally and checking the result is equivalent and slightly cleaner:

const parsedStatus = parseArrayParam(params.status);
if (parsedStatus && parsedStatus.every(s => typeof s === 'string')) {
  params = { ...params, status: parsedStatus as string[] };
}

The current code is correct — just slightly inconsistent with how the other handlers call parseArrayParam.

---

src/handlers/RecurringTaskHandler.ts — Frequency parsing

Good use of parseObjectParam. One edge case: if args.frequency is truthy but non-parseable (e.g., boolean true passed by an LLM), parseObjectParam returns undefined and the error says "frequency must be an object with a 'type' property". Accurate, though potentially confusing when the input was not a stringified object at all. Not a bug — just something to keep in mind if that error surfaces in user reports.

---

tests/parameterUtils.spec.ts — New utility tests

Strong coverage: 9 tests for parseObjectParam, 8 for parseArrayParam, and 5 for the JSON-stringification path in parseAutoScheduledParam. A few missing edge cases:

• parseObjectParam('{}') returns {} (empty object). Valid behavior (downstream validation catches missing fields), but undocumented.
• parseArrayParam('[]') returns [] (empty array). Same situation.
• parseArrayParam('[{"nested":"obj"}]') — the consumer's .every(v => typeof v === 'string') would reject this correctly, but there is no test for the mixed-type rejection path.

---

tests/handlers.recurring.spec.ts — Handler-level tests

The two new tests (stringified frequency and non-object frequency error) are exactly right.

Missing coverage: The analogous stringification paths in CustomFieldHandler (multiSelect value) and TaskHandler (status array) have no handler-level tests. The utility-level tests prove parseArrayParam works, but they do not verify the handler wires it up correctly or that the field === 'multiSelect' guard is enforced. Worth adding for completeness.

---

src/tools/ToolDefinitions.ts

The updated autoScheduled description is meaningfully better — the explicit "When the user specifies a start date, you MUST use the object form." is good prompt engineering.

Minor nit: the example uses "startDate": "2025-03-05", which is now a year in the past. A relative placeholder like "YYYY-MM-DD" or a clearly future date would avoid confusion for users reading the schema.

---

Summary

Area	Assessment
Core utility design	Good — symmetric, safe, well-typed
Handler integration	Correct; minor duplication in CustomFieldHandler and redundant guard in TaskHandler
Test coverage (utilities)	Good; a few edge-case gaps documented above
Test coverage (handlers)	RecurringTaskHandler covered; CustomFieldHandler and TaskHandler stringification paths untested
Backward compatibility	Fully preserved — native types pass through unchanged
Security	Safe — JSON.parse on string literals, no code execution risk
README / CHANGELOG	Accurate and complete

Good, targeted PR. The observations above are minor cleanup opportunities for a follow-up; nothing blocks the approach.