feat(mcp): import/move_copy mutates in shared cad_submit map - #64
feat(mcp): import/move_copy mutates in shared cad_submit map#64jeffglousher wants to merge 6 commits into
Conversation
cad_submit writes inbox/<seq>.json; the desktop applies via host::handle and publishes. MCP does not write model.json (no last-writer-wins).
Address Jack's three jackControls#60 blockers: 1. call_tool enforces session_read_only for direct mutates while attached (cad_submit / inspect / export stay allowed); writeback:true attach is rejected. Regression covers attach → direct mutate rejected → cad_submit accepted → detach restores headless mutation. 2. WindowPublisher.engine_revision is the authoritative OCC gate, advanced atomically with live engine apply and on immediate UI mutation notes (not heartbeat-debounce alone). Deterministic tests cover stale-base after UI note and two same-base ops. 3. Shared nbcad-mcp-mutate name→engine-method+payload map drives both cad_submit accept-list and Tauri inbox dispatch; unsupported names are rejected before enqueue; failed applies dead-letter to inbox/failed/ so the queue cannot wedge. Table-driven coverage of accepted mutates.
Stack assembly ToolSpecs (jackControls#62) onto the UI-owned apply / mcp-mutate branch (jackControls#60) so cad_submit accepts assembly_create_component, assembly_update_component, assembly_create_occurrence, assembly_update_occurrence, assembly_set_occurrence_pose, and assembly_set_occurrence_grounded while attached. assembly_document and assembly_solution stay read-safe inspect tools. Add Assembly focus pack and classifier/roundtrip coverage.
|
Stacked on #63 ( Merge after #60 (and ideally after #61 + #63). Not merging from here. |
jackControls
left a comment
jackControls
left a comment
There was a problem hiding this comment.
The new import/Move-Copy shared-map entries and focused tests look internally consistent, but the current PR is not approvable because its full stacked diff still contains the merge blockers from #61/#63:
- Assembly update schemas permit rename-shaped partial requests that default and overwrite body ownership, local transforms, parent, pose, visibility, and grounding.
- Inbox-applied assembly/non-solid changes fall back to
loadDocument(), which setsdirty: false; the ensuing store update also callsmcp_session_bridge_note_mutationafter the native apply already advancedengine_revision. - The ported
cad_scripttrace is not synchronized with snapshot replacement. Attach/refresh replace the in-memory document while the trace is neither reset nor seeded withcad_load_project_model, so it can be empty for the attached model or describe an earlier model. The regression does not replay the emitted calls on a fresh server.
Please land/fix the dependency PRs, then rebase this to a minimal import/Move-Copy map delta and retain an end-to-end replay test. I ran cargo test -p nbcad-mcp-mutate (5/5) and the full MCP suite (46/46); those green tests do not exercise the contracts above. Requesting changes; I did not merge.
Introduce ComponentDefinitionPatchDto / ComponentOccurrencePatchDto with optional fields (id required). Host update_component / update_occurrence merge omitted values instead of full-replacing with serde defaults. Rename-only MCP + assembly regression tests cover the wipe bug Jack flagged.
applyInboxNow no longer falls back to loadDocument() (which cleared dirty). Assembly ops get a targeted refreshAfterInboxApply with dirty:true; other non-solid results refresh live state the same way. While inboxApplying, the store subscription skips mcp_session_bridge_note_mutation so native apply's engine_revision bump is not double-counted.
Stack STEP import + solid move/copy ToolSpecs (jackControls#61) onto the fixed assembly submit-map tip (jackControls#63 @ 19800b9) so cad_submit accepts solid_import_step, solid_edit_import_step, solid_move_copy, and solid_edit_move_copy while attached. cad_script and cad_compare_solids stay read-safe control helpers (not inbox mutates). Upgrade cad_script to jackControls#61 @ 52266b2 behavior: attach/refresh seeds a cad_load_project_model baseline, and the e2e test actually replays the dumped script on a fresh CadServer (detach after attach so jackControls#60 session_read_only still holds). Bump MODELING_TOOL_COUNT and keep ToolSpec↔map sync.
348665a to
2958d67
Compare
|
Rebased onto #63 tip ( Minimal tip delta on top of assembly submit-map:
cc @jackControls — please re-review. Not merging. |
jackControls
left a comment
jackControls
left a comment
There was a problem hiding this comment.
Re-reviewed at 2958d67. The PR-local mutation-map additions and portable replay changes look sound. This stacked tip still inherits PR60's unresolved atomic revision race and stuck conflicted/malformed inbox head behavior, and its MCP check is failing on the inherited formatting diff. Please resolve/rebase those blockers before approval.
|
Closing as superseded by #60 @ After #60 rebased onto main ( No unique leftover worth rebasing — this PR’s tip only duplicated those map entries on top of the old #63 stack. |
2 participants
Summary
solid_import_step/solid_edit_import_stepandsolid_move_copy/solid_edit_move_copyin the sharednbcad-mcp-mutatemap socad_submitaccepts them while attached (samesolid_prepare_body_feature/solid_prepare_edit_body_featureengine methods as feat: MCP STEP import, forward tool-script dump, and solid compare #61).cad_scriptforward-trace, andcad_compare_solidsscene summary. Those two helpers are not mutates — they are control tools and are on the read-safe-while-attached list.feat/mcp-assembly-submit-map(feat(mcp): assembly mutates in shared cad_submit map #63 tip) so the allow-list grows monotonically after feat(mcp): UI-owned inbox apply (#11 slice) #60/feat(mcp): assembly mutates in shared cad_submit map #63 assembly entries.Merge order
Merge after #60 (and ideally after #61 + #63). Fork PRs target
jackControls/maindirectly because stacking against other fork heads is unreliable.Test plan
cargo test -p nbcad-mcp-mutatecargo testinmcp-server(46 passed), including:lookup_mutate/ classifier asserts for import + move_copytool_spec_mutates_match_shared_inbox_mapmcp_import_step_records_forward_scriptmcp_solid_move_copy_translates_bodycad_script_excludes_attach_refresh_after_snapshot_modeling(adapted for feat(mcp): UI-owned inbox apply (#11 slice) #60 attach read-only)Refs: #60 #61 #63
cc @jackControls