feat(mcp): UI-owned inbox apply (#11 slice)

[jeffglousher]

Summary

• Next [P0][architecture] Co-link MCP and UI to one live document session #11 MCP↔UI slice: UI-owned apply, not last-writer-wins model.json writeback (Jack removed that; not brought back).
• cad_submit (attached only) writes inbox/<seq>.json as { name, arguments, base_generation }. Does not mutate the MCP in-memory document.
• Stale base_generation → structured {code:"generation_conflict", writeback:false, session_mode:"ui_owned_apply", ...}. Submit of inspect/export/control is rejected (same deny-list idea as fix(mcp): reject mutations while snapshot-attached (tutor-safe #11 slice) #55 is_read_safe_while_attached). Headless (no attach) still mutates locally.
• apply_inbox_op in mcp-server/src/session.rs reads one inbox file, checks generation, calls host_apply, archives the op. Caller publishes the model.
• Tauri hook is actually called: mcp_session_bridge_apply_inbox dispatches on the live AppState (host::handle / solid-replay, same path as IPC). src/sessionBridge.ts polls it every 250ms from startSessionBridge() (already invoked from src/main.tsx). After apply, applySolidUpdate / loadDocument runs and the existing publisher writes the new snapshot.

Why

#55 (still open) rejects in-memory MCP mutations while attached. Acceptance still missing: MCP solid op appears in the UI document. Jack-happy rule: desktop/engine is the only writer of the live document. MCP may only submit.

Test plan

• cargo test --manifest-path mcp-server/Cargo.toml --bin nbcad-mcp snapshot -- --test-threads=1
• New tests (same command + names): attach_cad_submit_writes_inbox_without_mutating_memory, apply_inbox_helper_on_separate_manager_then_refresh_sees_body, stale_base_generation_is_generation_conflict_and_does_not_apply, cad_submit_without_attach_fails, headless_goldens_still_mutate_without_attach, inbox_write_and_stale_apply_are_generation_locked
• Full nbcad-mcp unit suite: 34 passed
• Desktop: attach MCP to a running UI session, cad_submit(solid_extrude|solid_mirror), confirm the body appears in the viewport, then cad_refresh sees the same snapshot
• Tauri session_bridge tests (this box has no GTK/pkg-config, so src-tauri did not compile here)

What is still missing for “MCP extrude visible in UI”

• A running desktop window that has published a session (this box cannot launch the Tauri UI).
• Agent must use cad_submit (not solid_extrude on the attached MCP copy). fix(mcp): reject mutations while snapshot-attached (tutor-safe #11 slice) #55 is still the in-memory lock; this PR does not merge it.
• After UI apply+publish, MCP must cad_refresh (no watch).
• Inbox dispatcher covers the main solid/sketch mutates; exotic MCP names fall back to host::handle by name and may need a mapping if they miss.

What this is not

• [P0][architecture] Co-link MCP and UI to one live document session #11 stays open. Not in-process shared memory, not OT/CRDT, not LWW writeback.
• Does not merge fix(mcp): reject mutations while snapshot-attached (tutor-safe #11 slice) #55 / fix: insert sketches and datums at the history cursor #49 / [follow-up] Finished-sketch list still appends after mid-history insert #52.

Refs

#11 #55

[jackControls]

I found three correctness blockers:

1. Attached direct mutations are still executable. The new classification at mcp-server/src/main.rs:790 is only used by cad_submit; call_tool never enforces it. An attached client can still call a mutating tool directly, change only its private SketchManager, and recreate the silent fork this design is meant to prevent. Please incorporate or depend on #55, return the structured session_read_only error for direct mutates while attached, and add a regression covering attach -> direct mutate rejected -> cad_submit accepted -> detach restores headless mutation.

2. The generation check at src-tauri/src/session_bridge.rs:443 races the live engine. The UI publishes after a 300 ms debounce, while inbox polling runs every 250 ms. A stale-base op can therefore be applied after a local UI mutation but before heartbeat.json advances; likewise, two queued ops with the same base generation can both apply before the first publish. Please validate and advance an authoritative backend revision atomically with the live engine apply, then add deterministic tests for both races.

3. cad_submit accepts every modeling mutate, but dispatch_inbox_on_engine only maps a subset. The fallback at src-tauri/src/session_bridge.rs:385 passes the MCP tool name to host::handle; for example sketch_add_line_locked requires engine method add_line_locked, so the queued op fails and remains first forever, blocking later operations. Please share/normalize the ToolSpec name-to-engine-method and payload mapping, reject unsupported names before enqueue, and ensure a failed op is surfaced/dead-lettered rather than wedging the queue. A table-driven test should cover every accepted mutate.

Requesting changes; I did not merge.

[jeffglousher]

Addressed the three CHANGES_REQUESTED blockers on b6ef8be (rebased on main with #58/#59):

1. Attach direct mutates — Ported fix(mcp): reject mutations while snapshot-attached (tutor-safe #11 slice) #55’s deny-path into call_tool: while attached, non-read-safe tools return structured session_read_only (writeback: false). cad_submit / inspect / export stay allowed; writeback: true attach is rejected. Regression: attach → direct mutate rejected → cad_submit accepted → detach restores headless mutation.

2. Generation race — WindowPublisher.engine_revision is now the authoritative OCC gate. It advances atomically with live engine apply under the publisher lock, and immediately on UI mutation via mcp_session_bridge_note_mutation (no 300 ms debounce). Heartbeat publish cannot lag behind. Deterministic tests cover (a) stale-base after UI note before publish and (b) two same-base ops where the second conflicts after the first advances.

3. Shared mutate map + no wedge — New crates/mcp-mutate (nbcad-mcp-mutate) shares name→engine-method+payload with both MCP ToolSpec/cad_submit and Tauri inbox dispatch (sketch_add_line_locked → add_line_locked, etc.). cad_submit rejects unsupported names before enqueue (unsupported_inbox_mutate). Failed applies dead-letter to inbox/failed/ so the queue cannot stick. Table-driven tests cover the accepted mutate set / sync with ToolSpec.

Tests: mcp-server cargo test (40 ok, including new lock+submit regression + ToolSpec/shared-map sync); nbcad-mcp-mutate unit tests; src-tauri session_bridge:: tests (11 ok, including both race cases + dead-letter).

Please re-review; not merging.

[jackControls]

Re-reviewed at b6ef8be. The earlier direct-mutation bypass is fixed, but two blocking correctness gaps remain. First, local UI mutations advance the session engine_revision only through a later fire-and-forget JavaScript noteEngineRevision call. Between native mutation completion and that second command, inbox apply can acquire the publisher lock and accept a stale-base operation. Please advance or check the authoritative revision in the same native critical section or transaction as the engine mutation. Second, the lowest conflicting or malformed inbox entry remains pending and is retried forever. A refreshed operation submitted at the next sequence cannot progress because the stale entry remains at the head. Please dead-letter or archive it, explicitly replace or rebase it, or otherwise resolve invalid and conflicting head entries. Current tests note mutation before queueing and explicitly expect a conflicting sequence to remain pending, so they do not cover these production failure windows.

[jeffglousher]

Addressed both remaining blockers on 30785fe (rebased onto latest upstream/main including #61 import/move_copy; stayed off #65).

1. Authoritative revision in the same native critical section

UI mutates no longer rely on a later fire-and-forget JS noteEngineRevision.

• New SessionBridgeState::run_ui_mutation holds the publisher lock across the live engine call (lock order: publisher → engine, same as inbox apply) and bumps engine_revision + heartbeat on success before releasing.
• Tauri mutating engine_command! / solid / project mutators go through that path; read-only commands are marked , read.
• Frontend store subscription no longer calls mcp_session_bridge_note_mutation (that JS gap was the race). Snapshot publish is unchanged.
• Inbox apply still advances revision under the same lock after a successful dispatch (no double-count from JS).

2. Stuck / conflicting / malformed head

apply_one_inbox_op now dead-letters to inbox/failed/ (and returns dead_lettered: true) for:

• generation_conflict (stale base)
• malformed JSON / missing fields
• unsupported mutate / engine failure

Pending queue advances so a later refreshed seq can apply. Tests no longer expect a conflicting head to remain pending forever.

Tests

• nbcad-mcp-mutate: 4 ok (incl. feat: MCP STEP import, forward tool-script dump, and solid compare #61 solid_move_copy / solid_import_step)
• nbcad-mcp: 43 ok
• src-tauri session_bridge::: 12 ok — including
  • ui_native_mutation_rejects_stale_base_without_js_note
  • inbox_generation_mismatch_is_dead_lettered_and_unblocks_queue
  • two_same_base_ops_second_is_dead_lettered_after_first_advances
  • malformed_inbox_head_is_dead_lettered_so_next_seq_applies

Please re-review; not merging. #65 untouched.

[jackControls]

Re-reviewed at 30785fe. The native mutation critical section and dead-letter queue fix the earlier blockers, and the focused bridge tests pass. Two P1 coherency races remain:

1. publishNow reserves a generation, then exports the live engine state, then writes the snapshot in three separate awaits. A UI mutation can complete after export but before write, advance engine_revision, and the stale export is still accepted and published at that current revision. An MCP client can then author against stale geometry and pass the base-generation check. The reservation needs to capture the engine revision and the write must reject/retry if it changed during export, or export/write must share the native publisher-to-engine critical section. Please add a deterministic mutation-between-export-and-write test.

2. Project-session bind/create/activate/drop bypass SessionBridgeState, while inbox dispatch always targets AppState.active_mut(). If an op was queued for tab A and the user activates retained tab B before the next debounced publish, the old base can still match and the op applies to B. Please bind the bridge to the active native project-session identity and reject mismatches, and advance/rebind it atomically on project-session transitions. Add an A-to-B switch test that proves an A op cannot mutate B.

Requesting changes until both target-coherency gaps are closed.

[jeffglousher]

Rebased onto affb1f0 (#62). P1s at 30785fe:

1. reserve now captures engine_revision; write skips with engine_revision_changed if a UI mutation landed during export (frontend retries). Test: mutation_between_export_and_write_rejects_stale_snapshot.
2. Bridge binds to the active native project-session identity under the publisher lock on bind/create/activate/drop. Per-tab MCP sessions are retained; apply rejects a bound/active mismatch. Test: inbox_op_for_tab_a_cannot_mutate_retained_tab_b.

f241739. Not merged.

[jeffglousher]

Rebased onto latest main (c46eea5, #65 project tabs / File menu + #66 i18n). Clean rebase — no conflicts, no bind retarget: native project-session id is still the tab id (bind/create/activate/drop unchanged). A-to-B test (inbox_op_for_tab_a_cannot_mutate_retained_tab_b) still valid against #65 retained-tab identity.

New tip: 5526bf0. cargo fmt clean; session_bridge 14, mcp-mutate 4, mcp-server 46 all green. Not merging.

[jeffglousher]

Rebased onto latest main (a0015f0, #67 constraint glyphs). Clean rebase — no conflicts. Kept session_bridge/inbox; took main for viewport/sketch (only overlap was src-tauri/src/lib.rs, auto-merged engine_delete_constraint onto the mutate wrapper). New tip: 25bdf45. mcp-mutate 4, mcp-server 46, session_bridge 14 green. Not merging.

[jackControls]

Thanks for the repeated rebases and for adding native project/session binding. I rechecked the current head (25bdf45) and reran the focused bridge and MCP suites. Two concurrency blockers still remain:

1. Publication reservation and write are not bound to the same project/session identity. The reservation response includes session_id and project_session_id, but the frontend write payload carries only the generation and exported model/focus. The backend then resolves publisher.active_mut() at write time. If tab A reserves generation N, tab B becomes active and also reserves generation N, and A's delayed export then writes, that payload can consume B's reservation and publish A's model into B's session. The inbox-side project binding does not protect this publication path. Please carry the reserved project/session identity through the write and reject a write if it no longer matches. A deterministic reserve-A → activate-B → reserve-B → write-A regression test would cover this.

2. Concurrent cad_submit calls can allocate the same inbox sequence and overwrite one operation. next_inbox_seq scans for max + 1, then the operation is written separately. Two MCP processes can choose the same sequence and both report success while one rename replaces the other. Please make sequence reservation/write exclusive (for example, an atomic create or per-session lock) and add a contention test proving every accepted operation gets a distinct durable entry.

The existing focused tests pass, but they do not exercise either interleaving. These need to be resolved before this bridge is safe for simultaneous tabs/agents.

[jeffglousher]

Addressed the two remaining concurrency P1s at 25bdf45 on 5447175. Not merging.

1. Publish write is bound to the reserved identity

publishNow now carries session_id + project_session_id from reserve through write. Native write resolves that reserved per-project publisher — never active_mut() at write time. Missing identity is rejected; reserved identity ≠ target is skipped (session_identity_mismatch). Frontend retries on that skip or engine_revision_changed.

Test: reserve_a_activate_b_reserve_b_write_a_does_not_publish_into_b — generation-only write errors; crossed A→B identity is skipped; A's delayed write publishes into A only; B's reservation stays intact.

2. Exclusive inbox sequence allocation

write_inbox_op treats next_inbox_seq as a hint and durable-allocates with OpenOptions::create_new (O_EXCL). Collision retries the next free seq (pending + applied + failed). Two cad_submit callers cannot share a sequence or overwrite while both report success.

Test: concurrent_inbox_alloc_gives_distinct_durable_entries — 16×8 contended writes → 128 distinct durable inbox files, each keeping its marker.

Tests

• nbcad-mcp-mutate: 4 ok
• nbcad-mcp: 47 ok (incl. contention)
• src-tauri session_bridge::: 15 ok (incl. reserve-A / activate-B / reserve-B / write-A)

Please re-review; not merging.

[jackControls]

Thanks for the update. I rechecked both prior P1 races: the write is now bound to the reserved project/session identity, and inbox allocation uses exclusive create-with-retry. The new cross-tab and concurrent-allocation tests exercise the exact failure modes, and the focused local tests plus all CI jobs pass. Approved.