fix: coerce LLM string numbers to integers before phase lookups in transition phase

[B-A-M-N]

Problem

The handleTransitionPhase MCP tool expects next_phase_id, completed_phase_id, and next_phase_ids to be numeric types. However, LLM models can occasionally generate string numbers in function call arguments — e.g., next_phase_id: "2" instead of next_phase_id: 2 — which causes strict-equality (===) phase lookups to fail silently.

Fix

Adds a coerceNumber() helper that safely converts string numbers to actual integers before the strict-equality phase lookups run. Non-numeric strings, empty strings, floats, and non-positive values pass through unchanged so downstream errors remain accurate.

Files Changed

• src/mcp/handlers/session-state-tools.js — added coerceNumber() and coercion logic
• claude/src/mcp/handlers/session-state-tools.js — generated mirror
• plugins/maestro/src/mcp/handlers/session-state-tools.js — generated mirror
• tests/unit/session-state-tools.test.js — unit tests for coerceNumber()

Generated files are in sync with source (zero-drift verified).

[josstei]

Review Summary

Thanks for tackling this -- LLMs emitting string-typed phase IDs is a real problem, and the fix is directionally correct. The coerceNumber approach is sensible. I have a few concerns that should be addressed before merging.

What works well

• Correctly identifies that string-typed IDs break === lookups against numeric phase IDs in session state
• Applies coercion to all three relevant parameters (completed_phase_id, next_phase_id, next_phase_ids)
• Non-numeric strings pass through unchanged so downstream error messages are preserved

Requested changes

1. Fix inaccurate PR description and comments (Low)
The PR says "before schema validation" but there is no runtime schema validation in the server pipeline. create-server.js:31 passes args directly to handlers. tool-registry.js registers schemas but never validates against them. The inputSchema is informational only (sent to LLMs via tools/list). The actual bug is === comparisons at lines 215, 226, and 237. The comment on line 170 should be corrected.

2. Tighten coerceNumber() to match existing patterns (Medium)
Number() accepts inputs that are never valid phase IDs: Number("") === 0, Number(" ") === 0, Number(true) === 1, Number("2.5") === 2.5. Phase IDs are always positive integers >= 1 per handleCreateSession (lines 87-89). The helper should use the same Number.isFinite() && Number.isInteger() && num > 0 pattern already established in this file.

3. Remove redundant type guards at call sites (Low)
The != null && typeof !== 'number' outer checks duplicate logic already inside coerceNumber(). Either simplify the call sites or the helper, not both.

4. Only edit src/, regenerate mirrors (Medium)
claude/src/ and plugins/maestro/src/ are generated by scripts/generate.js. Edit only src/mcp/handlers/session-state-tools.js and run npm run generate.

5. Add test coverage (Medium)
No tests exist for the string coercion path. The existing test at tests/transforms/mcp-session-pack.test.js:80-97 uses only integer IDs. At minimum, add cases for string IDs succeeding, invalid strings failing clearly, and array coercion.

Additional context

The MCP transport is JSON over stdio (maestro-server.js:62-81), so JSON natively distinguishes 2 (number) from "2" (string). This issue only arises when an LLM emits "next_phase_id": "2" as a string in its JSON output. The fix is worth having as a defensive measure, but the tighter validation will prevent silent mismatches from edge-case inputs.

---

Generated by Claude Code

[B-A-M-N]

Addressed the requested changes:

• Reworded the PR/comment to describe the actual issue (`===` phase-ID lookup mismatch), not schema validation
• Tightened `coerceNumber()` to only coerce finite positive integers — rejects `""`, `" "`, floats, zero, negatives
• Removed redundant outer type guards at call sites; `coerceNumber` is now idempotent
• Edited only `src/` and regenerated mirrors via `npm run generate`
• Added 13 tests covering string scalar IDs, string array IDs, invalid strings, and empty-string edge cases

All 13 new tests pass. Thanks for the detailed review.

[josstei]

Good to go!