fix(start-os,start-sdk): run effect-initiated actions under the caller's eventId

projects/start-sdk/CHANGELOG.md

@@ -9,8 +9,8 @@
   a plain value is no longer accepted. The run then answers that form, which is
   what lets a service run an action that takes input — another service's that
   `access` admits, via the new `packageId`, or its own. `prefill` seeds the
-  form. Underneath, `effects.action.getInput` accepts `prefill` and
-  `effects.action.run` accepts the `eventId` the form was opened under
+  form. Underneath, `effects.action.getInput` accepts `prefill`, and the form
+  and the run that answers it share the calling procedure's event id
 
 - **Breaking — a filled address lists the server's `.local` name whenever the
   user has it enabled, and `utils.mdnsResolvable` is removed.** `.local` was

projects/start-sdk/docs/src/actions.md

@@ -124,7 +124,7 @@ await sdk.action.run({
 })
 ```
 
-An action without input takes no `input`, and runs without a form. Calling the effects directly works the same way: `effects.action.run` answers a form only when it carries the `eventId` that `effects.action.getInput` returned, and an action with input refuses a run that names none.
+An action without input takes no `input`, and runs without a form. Calling the effects directly works the same way: the target keys the form `effects.action.getInput` opens by the calling procedure's event id, so the `effects.action.run` that answers it must come from the same procedure, one form at a time.
 
 ## Registering Actions
 

shared-libs/crates/start-core/src/service/action.rs

@@ -27,7 +27,7 @@ impl Handler<GetActionInput> for ServiceActor {
     }
     async fn handle(
         &mut self,
-        id: Guid,
+        event_id: Guid,
         GetActionInput {
             id: action_id,
             prefill,
@@ -38,7 +38,7 @@ impl Handler<GetActionInput> for ServiceActor {
         let container = &self.0.persistent_container;
         container
             .execute::<Option<ActionInput>>(
-                id,
+                event_id,
                 ProcedureName::GetActionInput(action_id),
                 json!({ "prefill": prefill, "caller": caller }),
                 Some(Duration::from_secs(30)),
@@ -53,7 +53,7 @@ impl Service {
     /// user and for the OS evaluating a task.
     pub async fn get_action_input(
         &self,
-        id: Guid,
+        event_id: Guid,
         action_id: ActionId,
         prefill: Value,
         caller: Option<PackageId>,
@@ -78,7 +78,7 @@ impl Service {
         }
         self.actor
             .send(
-                id,
+                event_id,
                 GetActionInput {
                     id: action_id,
                     prefill,
@@ -165,7 +165,7 @@ impl Handler<RunAction> for ServiceActor {
     }
     async fn handle(
         &mut self,
-        id: Guid,
+        event_id: Guid,
         RunAction {
             ref action_id,
             input,
@@ -218,7 +218,7 @@ impl Handler<RunAction> for ServiceActor {
         }
         let result = container
             .execute::<Option<ActionResult>>(
-                id.clone(),
+                event_id.clone(),
                 ProcedureName::RunAction(action_id.clone()),
                 json!({
                     "input": input,
@@ -253,14 +253,14 @@ impl Service {
     /// `None` for the user.
     pub async fn run_action(
         &self,
-        id: Guid,
+        event_id: Guid,
         action_id: ActionId,
         input: Value,
         caller: Option<PackageId>,
     ) -> Result<Option<ActionResult>, Error> {
         self.actor
             .send(
-                id,
+                event_id,
                 RunAction {
                     action_id,
                     input,

shared-libs/crates/start-core/src/service/effects/action.rs

@@ -7,7 +7,6 @@ use crate::action::{ActionInput, ActionResult, display_action_result};
 use crate::db::model::package::{
     ActionAccess, ActionMetadata, Task, TaskCondition, TaskEntry, TaskSeverity, TaskTrigger,
 };
-use crate::rpc_continuations::Guid;
 use crate::service::cli::ContainerCliContext;
 use crate::service::effects::prelude::*;
 use crate::util::serde::HandlerExtSerde;
@@ -116,13 +115,13 @@ async fn clear_actions(
 #[serde(rename_all = "camelCase")]
 #[ts(export)]
 pub struct GetActionInputParams {
-    #[serde(default)]
-    #[ts(skip)]
-    #[arg(skip)]
-    procedure_id: Guid,
     #[ts(optional)]
     #[arg(short, long, help = "help.arg.package-id")]
     package_id: Option<PackageId>,
+    #[serde(flatten)]
+    #[ts(skip)]
+    #[arg(skip)]
+    event: EventId,
     #[arg(help = "help.arg.action-id")]
     action_id: ActionId,
     #[ts(type = "Record<string, unknown> | null")]
@@ -133,13 +132,14 @@ pub struct GetActionInputParams {
 async fn get_action_input(
     context: EffectContext,
     GetActionInputParams {
-        procedure_id,
+        event,
         package_id,
         action_id,
         prefill,
     }: GetActionInputParams,
 ) -> Result<Option<ActionInput>, Error> {
     let context = context.deref()?;
+    let event_id = event.or_new();
     let prefill = prefill.unwrap_or(Value::Null);
     let caller = Some(context.seed.id.clone());
 
@@ -152,11 +152,11 @@ async fn get_action_input(
             .await
             .as_ref()
             .or_not_found(&package_id)?
-            .get_action_input(procedure_id, action_id, prefill, caller)
+            .get_action_input(event_id, action_id, prefill, caller)
             .await
     } else {
         context
-            .get_action_input(procedure_id, action_id, prefill, caller)
+            .get_action_input(event_id, action_id, prefill, caller)
             .await
     }
 }
@@ -166,17 +166,13 @@ async fn get_action_input(
 #[serde(rename_all = "camelCase")]
 #[ts(export, rename = "EffectsRunActionParams")]
 pub struct RunActionParams {
-    #[serde(default)]
-    #[ts(skip)]
-    #[arg(skip)]
-    procedure_id: Guid,
     #[ts(optional)]
     #[arg(short, long, help = "help.arg.package-id")]
     package_id: Option<PackageId>,
-    /// The `eventId` from the `get-input` whose form this input answers.
-    #[ts(optional)]
-    #[arg(long, help = "help.arg.event-id")]
-    event_id: Option<Guid>,
+    #[serde(flatten)]
+    #[ts(skip)]
+    #[command(flatten)]
+    event: EventId,
     #[arg(help = "help.arg.action-id")]
     action_id: ActionId,
     #[ts(type = "any")]
@@ -186,15 +182,14 @@ pub struct RunActionParams {
 async fn run_action(
     context: EffectContext,
     RunActionParams {
-        procedure_id,
+        event,
         package_id,
-        event_id,
         action_id,
         input,
     }: RunActionParams,
 ) -> Result<Option<ActionResult>, Error> {
     let context = context.deref()?;
-    let procedure_id = event_id.unwrap_or(procedure_id);
+    let event_id = event.or_new();
     let caller = Some(context.seed.id.clone());
 
     let package_id = package_id.as_ref().unwrap_or(&context.seed.id);
@@ -260,35 +255,34 @@ async fn run_action(
             .await
             .as_ref()
             .or_not_found(package_id)?
-            .run_action(procedure_id, action_id, input, caller)
+            .run_action(event_id, action_id, input, caller)
             .await
     } else {
-        context
-            .run_action(procedure_id, action_id, input, caller)
-            .await
+        context.run_action(event_id, action_id, input, caller).await
     }
 }
 
 #[derive(Clone, Debug, Deserialize, Serialize, TS)]
 #[serde(rename_all = "camelCase")]
 #[ts(export)]
 pub struct CreateTaskParams {
-    #[serde(default)]
+    #[serde(flatten)]
     #[ts(skip)]
-    procedure_id: Guid,
+    event: EventId,
     replay_id: ReplayId,
     #[serde(flatten)]
     task: Task,
 }
 async fn create_task(
     context: EffectContext,
     CreateTaskParams {
-        procedure_id,
+        event,
         replay_id,
         task,
     }: CreateTaskParams,
 ) -> Result<(), Error> {
     let context = context.deref()?;
+    let event_id = event.or_new();
 
     let src_id = &context.seed.id;
     let active = match &task.when {
@@ -312,7 +306,7 @@ async fn create_task(
                 {
                     service
                         .get_action_input(
-                            procedure_id.clone(),
+                            event_id.clone(),
                             task.action_id.clone(),
                             Value::Null,
                             None,
@@ -418,3 +412,71 @@ async fn clear_tasks(
         .result?;
     Ok(())
 }
+
+#[cfg(test)]
+mod test {
+    use imbl_value::json;
+
+    use super::*;
+    use crate::rpc_continuations::Guid;
+
+    fn envelope(params: Value) -> (Guid, Value) {
+        let event_id = Guid::new();
+        let mut params = params;
+        params["eventId"] = json!(event_id.as_ref());
+        (event_id, params)
+    }
+
+    #[test]
+    fn effect_params_take_the_calling_procedures_event_id() {
+        let (id, params) = envelope(json!({ "actionId": "attach", "prefill": null }));
+        let get: GetActionInputParams = imbl_value::from_value(params).unwrap();
+        assert_eq!(get.event.or_new(), id);
+
+        let (id, params) = envelope(json!({ "actionId": "attach", "input": { "a": 1 } }));
+        let run: RunActionParams = imbl_value::from_value(params).unwrap();
+        assert_eq!(run.event.or_new(), id);
+        assert_eq!(run.input, json!({ "a": 1 }));
+
+        let (id, params) = envelope(json!({
+            "replayId": "r",
+            "packageId": "tor",
+            "actionId": "attach",
+        }));
+        let task: CreateTaskParams = imbl_value::from_value(params).unwrap();
+        assert_eq!(task.event.or_new(), id);
+        assert_eq!(AsRef::<str>::as_ref(&task.task.action_id), "attach");
+    }
+
+    #[test]
+    fn cli_event_id_reaches_the_request() {
+        let id = Guid::new();
+
+        let run =
+            RunActionParams::try_parse_from(["run", "--event-id", id.as_ref(), "attach", "{}"])
+                .unwrap();
+        let sent = imbl_value::to_value(&run).unwrap();
+        assert_eq!(sent["eventId"], json!(id.as_ref()));
+        assert_eq!(
+            imbl_value::from_value::<RunActionParams>(sent)
+                .unwrap()
+                .event
+                .or_new(),
+            id
+        );
+
+        assert!(
+            GetActionInputParams::try_parse_from([
+                "get-input",
+                "--event-id",
+                id.as_ref(),
+                "attach",
+            ])
+            .is_err()
+        );
+
+        let unnamed = RunActionParams::try_parse_from(["run", "attach", "{}"]).unwrap();
+        assert!(unnamed.event.event_id.is_none());
+        assert!(imbl_value::to_value(&unnamed).unwrap()["eventId"].is_null());
+    }
+}

shared-libs/crates/start-core/src/service/effects/prelude.rs

@@ -6,12 +6,24 @@ pub use crate::prelude::*;
 use crate::rpc_continuations::Guid;
 pub(super) use crate::service::effects::context::EffectContext;
 
-#[derive(Debug, Clone, serde::Serialize, serde::Deserialize, Parser, TS)]
+// Identifies the procedure an effect call belongs to. The container runtime
+// sets it to the calling procedure's id; `action run --event-id` sets it to
+// the id `get-input` returned.
+// A service treats a call carrying the id of a procedure it is running as part
+// of that procedure, exempt from its conflicts.
+// Not a doc comment: clap prints one as the about text of every command that
+// flattens this.
+#[derive(Debug, Clone, Default, serde::Serialize, serde::Deserialize, Parser)]
 #[group(skip)]
 #[serde(rename_all = "camelCase")]
-#[ts(export)]
 pub struct EventId {
-    #[serde(default)]
-    #[arg(default_value_t, long, help = "help.arg.event-id")]
-    pub event_id: Guid,
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    #[arg(long, help = "help.arg.event-id")]
+    pub event_id: Option<Guid>,
+}
+impl EventId {
+    /// A fresh id for a call made outside any procedure.
+    pub fn or_new(self) -> Guid {
+        self.event_id.unwrap_or_default()
+    }
 }

shared-libs/crates/start-core/src/service/mod.rs

@@ -328,10 +328,10 @@ impl Service {
             .flatten_ok()
             .map(|a| a.and_then(|a| a))
             .try_collect()?;
-        let procedure_id = Guid::new();
+        let event_id = Guid::new();
         for action_id in tasks {
             if let Some(input) = self
-                .get_action_input(procedure_id.clone(), action_id.clone(), Value::Null, None)
+                .get_action_input(event_id.clone(), action_id.clone(), Value::Null, None)
                 .await
                 .log_err()
                 .flatten()
@@ -372,7 +372,7 @@ impl Service {
     async fn new(
         ctx: RpcContext,
         s9pk: S9pk,
-        procedure_id: Guid,
+        event_id: Guid,
         init_kind: Option<InitKind>,
         recovery_source: Option<impl GenericMountGuard>,
         init_progress: Option<crate::progress::PhaseProgressTrackerHandle>,
@@ -417,7 +417,7 @@ impl Service {
         service
             .seed
             .persistent_container
-            .init(service.weak(), procedure_id, init_kind)
+            .init(service.weak(), event_id, init_kind)
             .await?;
         service.recheck_tasks().await?;
         if let Some(recovery_guard) = recovery_guard {
@@ -698,7 +698,7 @@ impl Service {
         crate::volume::ensure_volume_root(&manifest.id).await?;
         let developer_key = s9pk.as_archive().signer();
         let icon = s9pk.icon_data_url().await?;
-        let procedure_id = Guid::new();
+        let event_id = Guid::new();
         let (finalization_progress, overall_progress) = match progress {
             Some(InstallProgressHandles {
                 finalization_progress,
@@ -709,7 +709,7 @@ impl Service {
         let service = Self::new(
             ctx.clone(),
             s9pk,
-            procedure_id.clone(),
+            event_id.clone(),
             Some(kind),
             recovery_source,
             finalization_progress,