feat(mcp): assembly joint mutates in shared cad_submit map

[jeffglousher]

Summary

• Register assembly_create_joint / assembly_update_joint in crates/mcp-mutate so cad_submit accepts them while attached.
• Port feat(mcp): host-neutral assembly joint create/update #68 ToolSpecs, Assembly disclosure tags, and the revolute create/update/query golden.
• assembly_document / assembly_solution stay inspect-only (not in the map).

Stack

Branched from #60 tip 25bdf45. Merge after #60 and #68.

Test plan

• cargo test -p nbcad-mcp-mutate
• cargo test in mcp-server (incl. assembly_joint_create_update_query_roundtrip, ToolSpec↔map sync, classifier)

[jeffglousher]

Merge after #60 and #68. Joints in the shared cad_submit map; document/solution stay inspect-only.

[jackControls]

Thanks for stacking the joint mutates into the shared map. The map entries, disclosure tags, and host-neutral create/update round-trip look sound, but I found one PR69-specific blocker and three blockers carried by the stacked base:

1. P1 — attached joint mutations are marked saved. assembly_create_joint and assembly_update_joint return a joint DTO rather than a solid update. applyInboxNow() therefore falls through to loadDocument(), which unconditionally sets dirty: false. The joint appears in the live project, but closing the tab/app can skip the unsaved-changes warning and lose it. Please use a mutation-aware assembly refresh that preserves dirty state and add an attached cad_submit integration test covering joint visibility and dirty: true after both create and update.

2. P1 — inherited from PR60: cross-tab publish identity is lost between reservation and write. The reservation returns session/project identity, but the write payload only carries generation. Native write handling then targets whichever project is active at write time. A tab switch between export and write can publish one tab's model into another tab's session. Please carry and validate the reserved session/project identity through the write.

3. P1 — inherited from PR60: concurrent cad_submit calls can allocate the same inbox sequence. Sequence allocation scans for max + 1, then writes separately. Two submitters can choose the same sequence and one atomic rename can replace the other while both calls report success. Please allocate with an exclusive create/lock or another atomic reservation mechanism.

4. P2 — copied from PR68: advertised joint schemas reject valid serialized DTOs containing null. Optional fields including source_surface_frame, primary limits, and advanced limits serialize as null, while their schemas accept only objects. Passing a created or queried joint back to assembly_update_joint can therefore fail protocol-level validation. The current round-trip test calls CadServer::call_tool directly and bypasses input-schema validation. Please allow object-or-null for these fields and add a schema-validating round-trip test.

I recommend fixing item 1 here, then rebasing onto corrected PR60 and PR68. This PR also needs the assembly-refresh behavior from PR63 (including its stale-preview correction), or an equivalent focused refresh, so attached joint changes update the UI without resetting saved state.

Focused local checks passed: nbcad-mcp-mutate 5/5, MCP server 47/47, and desktop session bridge 14/14.

[jeffglousher]

Addressed item 1 (local only; not merging). Did not rebase onto #60/#68 — those tips are not yet carrying the identity/seq/schema fixes.

P1 — attached joint mutations marked saved
assembly_create_joint / assembly_update_joint return a joint DTO (no scene+document), so applyInboxNow() no longer falls through to loadDocument() (which set dirty: false). Ported #63 refreshAfterInboxApply:

• assembly ops: targeted refresh with dirty: true
• other non-solid results: live refresh with dirty: true
• solid updates still go through applySolidUpdate (already dirty:true)
• stale-preview correction: assembly refresh now clears jointMotionPreview (and jointPreviewSolution / mechanismPreview) so the viewport cannot keep rendering an old motion pose over the new assemblySolution

Test: attach_cad_submit_joint_create_update_visible_and_dirty — attached cad_submit create then update; after each apply+refresh the joint is visible and the host result is a joint DTO (the path that must keep dirty: true).

Checks: cargo fmt --all -- --check clean (root + mcp-server + src-tauri). nbcad-mcp-mutate clippy -D warnings clean, 5/5 tests. mcp-server 48/48. Did not rebase.

Please re-review. Still merge-after-#60 and #68.

[jeffglousher]

Item 1: attached assembly_create_joint / assembly_update_joint no longer fall through to loadDocument() (dirty: false).

applyInboxNow still applies solid updates via applySolidUpdate (already dirty: true). Joint DTOs have no scene+document, so they now take refreshAfterInboxApply — targeted assembly refresh with dirty: true. That path also clears jointMotionPreview (viewport otherwise prefers the stale preview over the refreshed assemblySolution).

Test: attach_cad_submit_joint_create_update_visible_and_dirty — attached cad_submit create and update; joint visible after each apply; host result is a joint DTO (the dirty: true refresh path, not loadDocument).

Also on this push (not waiting for #60 identity/seq — nobs-11-apply still 25bdf45):

• Cherry-picked feat(mcp): host-neutral assembly joint create/update #68 3a071a6 so advertised joint schemas accept object-or-null (assembly_joint_query_validates_against_advertised_update_schema).
• Inbox solid-update check matches feat(mcp): assembly mutates in shared cad_submit map #63 (no extra helper).

Please re-review item 1. Items 2–3 stay with corrected #60.

[jeffglousher]

Rebased onto #60 5447175 via #63 c7aba14.

Force-with-lease push: af2e670 → 06cad51. Joint MUTATES, dirty-refresh (incl. stale jointMotionPreview clear), object-or-null schemas, attach dirty test, and schema-validating round-trip kept. One conflict in src/store/appStore.ts (kept the #69 jointMotionPreview: null clear).

Local: cargo fmt clean; cargo test -p nbcad-mcp-mutate 6/6; mcp-server cargo test 51/51.

[jeffglousher]

Rebased onto #60 5447175 + #68 schemas (via new #63 tip c7aba14).

Kept unique leftover: joint MUTATES, dirty-refresh (with jointMotionPreview: null), object-or-null advertised fields, attach joint dirty test, schema-validating round-trip.

Local checks: rustfmt on touched; nbcad-mcp-mutate 6/6; mcp-server 51/51. Not merging.

[jackControls]

Thanks for the updates. I rechecked the full stacked head: the publisher/inbox races are fixed, joint create/update inbox results preserve dirty state, targeted assembly refresh clears all stale previews (including jointMotionPreview), and the nullable joint schema round-trip is covered. The focused joint integration test and all platform/MCP CI jobs pass. Approved.

The merge-base changed after approval.

[jeffglousher]

Rebased onto new #63 tip e94e27e (stale jointMotionPreview clear + source-lock). Then an adversarial pass on the stacked joint+inbox+attach path.

New tip: 26785fe

Real bugs / contract locks

• cad_submit leaked into cad_script. Successful cad_submit was recorded as a portable modeling op. It is session-control (inbox write) and is now skipped like attach/refresh/detach. After apply+refresh+detach, joints appear in the cad_load_project_model baseline model_json — not as reconstructed assembly_create_joint calls. cad_script stays rejected while attached (existing contract; not invented).
• Malformed joint apply could wedge the MCP helper queue. Native apply already dead-lettered dispatch failures; session::apply_inbox_op left a failed head pending. Failed/unreadable joint payloads now move to inbox/failed/ so the next seq can apply.
• UpdateJointRequestDto is replace-all, not a patch. Omitted optional fields (limits, source_surface_frame, …) deserialize as null and clear. Tool description now says so. Full queried-DTO rename preserves limits; omitted limits clear. No host patch DTO invented.

Already sound (locked with tests)

• Attached create → update → query; dirty stays true (joint DTO ≠ scene+document, so loadDocument is not the apply path).
• Explicit null optional fields + tools/list schema validation on the attached update.
• Two concurrent cad_submit joint ops get distinct inbox seqs (create_new reservation).
• Tab A vs B / exclusive seq stay on the feat(mcp): UI-owned inbox apply (#11 slice) #60 identity path (no joint-specific hole).
• Stale jointMotionPreview cleared on assembly inbox refresh (e94e27e).

Tests

cargo test -p nbcad-mcp-mutate 6/6. mcp-server cargo test 57/57, including:

• attach_cad_submit_joint_create_update_visible_and_dirty
• attach_cad_submit_joint_null_fields_schema_and_script_baseline
• attach_cad_submit_two_joint_ops_get_distinct_seqs
• attach_malformed_joint_payload_is_dead_lettered
• assembly_update_joint_full_record_rename_preserves_limits
• assembly_update_joint_omitted_limits_clear_on_replace_all
• assembly_inbox_refresh_clears_joint_motion_preview

fmt on touched. Not merging until Approved.

[jeffglousher]

Second adversarial pass at 7f7fc15 (stacked on #63 / main, #60 merged). Local only — did not merge. Off Jack UI.

No host bug. Replace-all is still the contract (not a patch). Tightened the advertised assembly_update_joint description + joint schema text so id+name-only cannot be read as a rename patch.

New angles locked:

1. id+name-only — schema-invalid (required kind/connectors); host serde rejects; existing joint name+limits unchanged.
2. create then update before cad_refresh — joint not lost, inbox results stay DTOs (dirty:true path), preview-clear source still present.
3. detach mid-inbox — apply stays on a separate host; detached manager is not mutated (no fork); re-attach sees the published joint.
4. explicit JSON null vs omitted keys — both specified; both clear optional limits.
5. missing/wrong occurrence (component instance) — typed host error, dead-letter, queue unblocked (cad_set_document_name applies after).
6. headless vs attached — direct assembly_create_joint works when not attached; cad_submit without attach stays not_attached; while attached, direct is session_read_only and only inbox is accepted.

#68 is not APPROVED+MERGEABLE — left it alone (parent merges).

Local: nbcad-mcp-mutate 6/6, nbcad-mcp 63/63. rustfmt on touched. clippy -D warnings still hits pre-existing lints in this file (chunks_exact_to_as_chunks, manual_contains, unused-in-bin session helpers) — none introduced by this pass.

Re-requesting review.

[jeffglousher]

Third adversarial pass at fa4582e (stacked on 7f7fc15). Local only — did not merge. Off Jack UI.

One real bug, helper now matches native. session::apply_inbox_op left a generation-conflicted head pending after the first same-base apply+publish. Native already dead-letters that case (two_same_base_ops_second_is_dead_lettered_after_first_advances). Helper now dead-letters generation_conflict with the reason so later seqs can run. No API change.

New angles locked:

1. assembly_solution after cad_submit create — apply+refresh; solution is a DTO (body_poses / occurrence_poses / solved / diagnostics) and includes both jointed occurrence ids. Empty-graph and unsolved still return a DTO (no crash).
2. Two different joints on the same occurrence pair — revolute then slider both exist; follow-up cad_set_document_name still applies (queue not wedged). Solution after both still a DTO.
3. Rapid create+update same base_generation — both queue with distinct seqs; create applies; leftover update dead-letters with generation_conflict (never silent drop); rebased follow-up applies; document still shows the created name.
4. Inbox file malformed JSON ({not-json, not just bad joint fields) — dead-letter keeps raw bytes; next op applies.
5. assembly_update_joint occurrence-id swap — swap-only ids (bodies stay) is a typed reject (occurrence / does not contain / binding); original ids unchanged. Swapping both connectors (bodies + occurrence ids) is legal; document query matches.

Local: nbcad-mcp-mutate 6/6, nbcad-mcp 70/70. rustfmt on touched.

Re-requesting review.

[jeffglousher]

Fifth adversarial pass at ff30e4c (stacked on defc843). Local only — did not merge. Off Jack UI.

No host bug. Remaining applyInboxNow / joint-adjacent result shapes already keep dirty:true. Joint DTO, component, occurrence, and assembly-document returns have no scene+document, so they take refreshAfterInboxApply (targeted assembly_* branch). That path already nulls jointPreviewSolution, jointMotionPreview, and mechanismPreview (viewport prefers preview.solution over assemblySolution). Double applyInboxNow of an already-archived seq is a no-op: native applied:false / empty, helper no pending inbox op, JS returns before any store write.

New angles locked:

1. applyInboxNow loadDocument fallthrough — source lock: no loadDocument(; !applied returns before applySolidUpdate / refreshAfterInboxApply; non-solid results call refreshAfterInboxApply(result.name).
2. mechanismPreview / jointPreviewSolution leftover — assembly inbox refresh must contain all three *: null plus dirty: true. Viewport order is jointPreview → mechanismPreview → jointMotionPreview → assemblySolution.
3. Double apply of the same already-applied seq — helper: second apply_inbox_op does not call host. Native: second apply_one_inbox_op is applied:false / empty, same document name, same engine_revision.

No TS/vitest harness; locked with existing Rust include_str + apply tests.

Local: nbcad-mcp-mutate 6/6. mcp-server: apply_inbox_now_never_falls_through_to_load_document, assembly_inbox_refresh_clears_joint_motion_preview, already_applied_inbox_seq_second_apply_is_noop. rustfmt on touched. src-tauri native no-op test added (box cannot link Tauri resources here).

Re-requesting review.

[jeffglousher]

Sixth adversarial pass at 057bff1 (stacked on ff30e4c / #63 e94e27e). Local only — did not merge. Off Jack UI.

No host bug. applyInboxAll does not exist; leftover refreshAfterInboxApply is still the single #63 path (dirty:true, no loadDocument, clears jointPreviewSolution / jointMotionPreview / mechanismPreview). Post-#60 main still uses loadDocument; #69 leftover matches #63, no third path.

New angles locked:

1. applyInboxAll vs applyInboxNow — source lock: no applyInboxAll / apply_inbox_all / applyAllInbox that could reintroduce loadDocument or skip the already-applied early return. Leftover refreshAfterInboxApply is defined once.
2. cad_refresh vs cad_load_project_model — after inbox create+update, attached cad_refresh and a fresh cad_load_project_model of the published model see the same joint ids, names, connectors, and limits. While attached, cad_load_project_model stays session_read_only.
3. joint inbox on a part — one-body document, schema-valid same-body joint: typed host reject (different occurrences), dead-lettered, no ghost / next_joint_id unchanged, queue not left pending.

Local checks: rustfmt on touched; nbcad-mcp-mutate 6/6; MCP source-lock + new parity/part tests. Not merging.

[jeffglousher]

Seventh adversarial pass at b9a5853 (stacked on 057bff1 / #63 e94e27e). Local only — did not merge. Off Jack UI.

Two real holes, both locked.

1. Missing heartbeat generation wedged leftover apply. session::apply_inbox_op returned generation_conflict when heartbeat.json generation was missing/unreadable but left the head pending. Native apply locks on in-memory engine_revision and never waits on the file. Helper now dead-letters that conflict so later seqs can run. Age-only stale heartbeat (age > HEARTBEAT_STALE_MS) with a matching generation still applies on both leftover and native — listing staleness is not a writer lock.

2. JS throw after native apply skipped publish. applyInboxNow called scheduleSessionBridgePublish() only after leftover refreshAfterInboxApply / applySolidUpdate succeeded. Native had already archived the seq and bumped engine_revision. A leftover refresh throw swallowed publish, so cad_refresh could miss the live joint. Publish now lives in a finally after applied:true. Next applyInboxNow stays a no-op on the archived seq (already locked). No loadDocument.

Also locked (no host change): leftover vs native error class + inbox/failed archive for generation_conflict / unsupported; extra unknown JSON fields on create/update do not stick or change the DTO; apply is lowest-pending-seq-first.

Focused local checks: nbcad-mcp-mutate 6/6; leftover session tests 12/12; new MCP locks + prior joint locks green; native native_apply_uses_engine_revision_not_heartbeat_file ok. rustfmt on touched files.

[jeffglousher]

Eighth adversarial pass at dee66f5 (stacked on b9a5853 / #63 e94e27e). Local only — did not merge. Off Jack UI.

No host bug. Queue unwedge after a failed head was already the leftover+native contract (dead-letter, then next applyInboxNow / apply_inbox_op takes the lowest remaining pending — not apply-all). Finally publish still writes reserved session_id + project_session_id (never active_mut()). Reattach-then-apply does not run pending against the wrong live document.

New angles locked:

1. Queue unwedge after failed seq — leftover generation_conflict on seq 1 then seq 2 applies. Native already locked malformed / conflict / unsupported / host-fail → next seq. JS dead-letter returns before finally publish so the next poll can take seq 2.
2. Identity-bound publish after the finally move — publishNow still carries reservation.session_id + reservation.project_session_id; native write_for_window resolves reserved identity (session_identity_mismatch), never active_mut().
3. Reattach / detach with leftover pending — detach, reattach same session, then apply: separate host only; attached manager stays clean until cad_refresh. Detach, attach a different session, then apply A's pending: B's manager/inbox stay clean; A's published model gets the joint. Native: pending on A is blocked while B is active, then applies against A after switch-back (B unchanged).
4. Bonus — occurrence rename after joint create keeps occurrence ids (not display names). Whitespace-only joint name (minLength: 1 accepts " "; host validate_joint treats trim-empty as typed reject) dead-letters and unblocks seq 2. No max-length invented.

Jack review comments: none new on tip b9a5853 (prior CHANGES_REQUESTED already addressed; later approve was on 06cad51 and dismissed by later pushes).

Local: nbcad-mcp-mutate 6/6; leftover session tests 12/12; native session_bridge 19/19; new MCP locks + prior detach/joint unwedge green. rustfmt on touched.

Re-requesting review.

[jeffglousher]

Ninth adversarial pass at e496b3d (stacked on dee66f5 / #63 e94e27e). Local only — did not merge. Off Jack UI.

No host bug. Leftover refreshAfterInboxApply is still the single #63 path (dirty:true, no loadDocument, three preview nulls). #69 leftover apply_inbox_op remains a strict superset (pass 7 missing-hb / stale-gen / unsupported / host-fail dead-letter). applyInboxNow still publishes in finally after applied:true. No third leftover path. Did not open a #63-only PR.

New angles locked:

1. Gone occurrence after create — create joint (applied), absorb one auto-promoted occurrence away, then inbox update and a second create that name the gone occ: typed reject, dead-letter, no ghost / next_joint_id unchanged, original joint keeps the absorbed occ, seq 4 name still applies.
2. Unknown joint id update — replace-all update of id 99999: typed does not exist, dead-letter, existing joint unchanged, later seq applies.
3. assembly_delete_joint honesty — tool does not exist (host AssemblyDocumentDto::delete is body-delete cleanup only). Locked out of ToolSpec, shared mutate map, and disclosure. Docs never claimed it.

O_EXCL double submit — already locked on #60/#63/#69 (write_inbox_op create_new + concurrent_inbox_alloc_gives_distinct_durable_entries). cad_submit uses that helper. Did not duplicate.

Jack review comments: none new on tip dee66f5 (prior CHANGES_REQUESTED already addressed; later approve was on 06cad51 and dismissed by later pushes).

Local: nbcad-mcp-mutate 6/6 clippy -D warnings clean; leftover session tests 12/12; new MCP locks green. rustfmt on touched. mcp-server package clippy -D warnings still hits pre-existing Jack/UI lints (unused session helpers outside tests, chunks_exact, manual_contains) — not touched.

Re-requesting review. Not merging.

[jeffglousher]

Tenth adversarial pass at fad8dca (stacked on e496b3d / #63 e94e27e). Local only — did not merge. Off Jack UI.

No host bug. Host delete is still body-delete cleanup (remove_joints_for_deleted_bodies on solid_delete_feature commit). Joints that reference the deleted body are removed; unrelated joints and their occurrence ids stay. Component structure is retained on purpose (reusable definitions / placement). Do not invent assembly_delete_joint.

Body-delete vs joints

1. Applied joint, then inbox solid_delete_feature of a connector body — leftover applyInboxNow takes applySolidUpdate (scene+document). Host cleanup drops the joint (no ghost, no stale joint occ). Leftover source-lock: applySolidUpdate keeps dirty:true, nulls the three previews, re-reads assemblyDocument/assemblySolution, and drops selectedJointId if the joint is gone.
2. Pending inbox create that names an occ, then live body-delete of that occ (same generation), then apply — typed host reject, dead-letter, no ghost, seq 2 applies.
3. Pending replace-all update after the joint's own body was deleted — cleanup already dropped the joint; update typed-rejects and does not resurrect it. Seq 3 applies.
4. Delete a body that is not part of the joint — joint and its occurrence ids unchanged (host + inbox).

Schema edges (already defined; not invented)

• Limits: required min/max numbers, no value range. min>max is schema-valid + typed host reject (invalid motion limits).
• Unicode joint name (ヒンジα-1) is schema-valid and persists through inbox apply + inspect. No name pattern.
• Inbox JSON that is valid JSON but the wrong tool name (assembly_delete_joint / inspect) is unsupported inbox mutate at apply; cad_submit still rejects those at submit.

Jack: no new inline comments. Latest review is the earlier dismissed approval. Not merging.

Local: rustfmt on touched; nbcad-mcp-mutate 6/6; mcp-server 95/95; assembly 33/33. Re-requesting review.

[jackControls]

valid but incorrectly shaped inbox JSON such as [] gets archived without its error reason.