Conversation
There was a problem hiding this comment.
Overall this looks great. I confirmed everything builds (happ and host), unit and sweetests all pass, and app runs correctly (via npm start).
I do have two suggested changes:
MutationAction Makes False Promise
The TransactionAction shape in this PR diverges from the current MAP Commands spec by introducing a second level of action nesting (LookupAction / MutationAction) instead of keeping the transaction action surface flat. See transaction_command.rs, transaction_wire.rs, and the tests that lock this shape in place: wire_serde_tests.rs and dispatch_tests.rs.
I agree the grouped shape has some local organizational appeal, but I don’t think this is a good place to vary from the spec.
Reasons:
MutationActionintroduces a semantic signal the model cannot actually guarantee. Some Dance operations may be mutating, and Dance is intentionally extensible, so nesting some actions underMutationActionimplies a CQRS boundary that is not structurally true.- That makes enum structure look like a policy/classification mechanism, when the spec says policy should be enforced by runtime descriptors and lifecycle rules, not inferred from command naming or ingress-visible shape.
- My recommendation is to keep TransactionAction aligned with the flat spec for this phase, and revisit sub-grouping later only if we can justify it without introducing misleading semantic guarantees.
Setup Failures Masked
Runtime initialization currently proceeds even after provider setup failure or when no conductor client was captured, which leaves the app with a partially functional MAP runtime instead of clearly unavailable runtime state. run_complete_setup() logs setup failure and continues into initialize_runtime(), and initialize_runtime() unconditionally builds Runtime from init_client_runtime(initiator) even when initiator is None. That contradicts the comment saying the runtime should remain None without a conductor client and will produce confusing behavior for commands that depend on conductor-backed capabilities. See app_builder.rs and app_builder.rs
…ice_ready with RuntimeState
MAP Commands — Phase 2.1 — Command Contract and Runtime Binding Seam (v0)
Closes #404.
Summary
Establishes the structural command contract and Runtime binding seam defined by the MAP Commands Specification. Introduces a new
map_commandscrate implementing the full wire/domain separation, aRuntimedispatch boundary, and Tauri integration — all without touching legacy receptor paths.What was built
map_commandscrate — new crate housing the full command type hierarchy:MapCommandWire,SpaceCommandWire,TransactionCommandWire,HolonCommandWire) with serde andbind()implementationsMapCommand,TransactionCommand,HolonCommand,LookupAction,MutationAction,ReadableHolonAction,WritableHolonAction) — no wire dependencies, no serializationMapIpcRequest/MapIpcResponseIPC envelope typesMapResult/MapResultWirewithFrom<MapResult>conversionRuntime dispatch sandwich —
Runtime::dispatch(MapIpcRequest) -> MapIpcResponse:RuntimeonlyRuntimeSessionowns transaction lifecycle (begin_transaction,get_transaction,remove_transaction) via anArc<HolonSpaceManager>dispatch_space,dispatch_transaction,dispatch_holon) operate on domain types only; non-implemented commands returnNotImplementedBeginTransactionandCommitexecute end-to-endTauri integration —
dispatch_map_commandTauri command:RuntimeState = RwLock<Option<Runtime>>managed state, initialized duringrun_complete_setupConductorClientStateshares theArc<HolochainConductorClient>between the receptor andinitialize_runtime()TrustChannelconstructed from the conductor client and passed toinit_client_runtime()as theDanceInitiatorholons_client— addedinit_client_runtime()returningArc<HolonSpaceManager>without opening an implicit transaction (transaction lifecycle owned byRuntimeSession)Tests
Runtime::dispatchend-to-end: transaction creation, uniqueness, unimplemented command shape, invalidtx_idbinding failurecargo check -p conductoracleanNot in scope
CommandDescriptorpolicy enforcement — Phase 2.2ClientSession/ undo-recovery integration (pending PR 412 recovery and transactional snapshots #418) — Phase 2.2