fix: use snake_case for model-facing tool inputs - #303
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (14)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe change standardizes MCP tool inputs on snake_case names, updates handlers and instructions, adds schema and runtime coverage, and centralizes historical review payload construction. ChangesTool input naming and documentation
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to MCP tool inputs now consistently use snake_case while internal and structured-output identifiers remain unchanged. Workspace-scoped and historical review calls use the updated payload shape, with no current merge-blocking risk identified. Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 10 files. (4 skipped: 4 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR consistently changes model-facing MCP tool inputs from camelCase to snake_case while preserving camelCase internal objects, structured outputs, and workspace state.
Confidence Score: 5/5The PR appears safe to merge; the renamed MCP inputs are consistently mapped at the boundary and covered by schema and runtime tests. No actionable failures remain: internal camelCase contracts and structured outputs are preserved, all examined model-facing schemas and callers use snake_case, and historical review restoration matches the updated server contract.
|
| Filename | Overview |
|---|---|
| src/server.ts | Renames shared workspace and worktree inputs, updates instructions, and preserves existing internal and output contracts. |
| src/tool-surfaces/claude.ts | Converts Claude workspace, edit, and working-directory inputs to snake_case with explicit internal mappings. |
| src/tool-surfaces/codex.ts | Converts patch and process-control inputs to snake_case while retaining camelCase process-session APIs and outputs. |
| src/artifact-tools.ts | Changes artifact workspace selection to workspace_id without altering download validation or publication behavior. |
| src/ui/tool-result.ts | Updates historical review reload requests to match the new show_changes input contract. |
| src/server.test.ts | Adds recursive schema naming checks and end-to-end coverage for representative Claude and Codex calls. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A["open_workspace output<br/>workspaceId"] --> B["Model-facing MCP call<br/>workspace_id"]
B --> C["Tool handler boundary"]
C --> D["Internal domain API<br/>workspaceId"]
D --> E["Structured output<br/>camelCase preserved"]
E --> F["Subsequent MCP input<br/>snake_case"]
Reviews (1): Last reviewed commit: "fix: align workspace tool callers with s..." | Re-trigger Greptile
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
1 participant
This addresses the model-facing input naming part of #297. ChatGPT can mangle camelCase tool fields such as
yieldTimeMs, so DevSpace now exposes snake_case names at the MCP boundary while keeping its internal/domain objects and structured outputs in their existing camelCase form.The change applies the same boundary consistently across the core, Claude, Codex, and artifact tool surfaces. It also updates the historical
show_changesUI call andopen_workspaceinstructions so callers use the returnedworkspaceIdasworkspace_idon subsequent tool calls. The intermittent MCP disconnects described in #297 are intentionally not part of this PR.I verified the current branch against the latest
main: the full test suite passes (109 tests, 104 passed, 5 platform-specific skips), TypeScript typechecking passes, and the production build completes successfully.Summary by CodeRabbit