diff --git a/projects/start-sdk/CHANGELOG.md b/projects/start-sdk/CHANGELOG.md index e39fbcf7c8..f354c538c4 100644 --- a/projects/start-sdk/CHANGELOG.md +++ b/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 diff --git a/projects/start-sdk/docs/src/actions.md b/projects/start-sdk/docs/src/actions.md index 4fb6abadd9..5b2242454e 100644 --- a/projects/start-sdk/docs/src/actions.md +++ b/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 diff --git a/shared-libs/crates/start-core/src/service/action.rs b/shared-libs/crates/start-core/src/service/action.rs index 7862c2a79a..b581a1c91b 100644 --- a/shared-libs/crates/start-core/src/service/action.rs +++ b/shared-libs/crates/start-core/src/service/action.rs @@ -27,7 +27,7 @@ impl Handler for ServiceActor { } async fn handle( &mut self, - id: Guid, + event_id: Guid, GetActionInput { id: action_id, prefill, @@ -38,7 +38,7 @@ impl Handler for ServiceActor { let container = &self.0.persistent_container; container .execute::>( - 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, @@ -78,7 +78,7 @@ impl Service { } self.actor .send( - id, + event_id, GetActionInput { id: action_id, prefill, @@ -165,7 +165,7 @@ impl Handler for ServiceActor { } async fn handle( &mut self, - id: Guid, + event_id: Guid, RunAction { ref action_id, input, @@ -218,7 +218,7 @@ impl Handler for ServiceActor { } let result = container .execute::>( - 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, ) -> Result, Error> { self.actor .send( - id, + event_id, RunAction { action_id, input, diff --git a/shared-libs/crates/start-core/src/service/effects/action.rs b/shared-libs/crates/start-core/src/service/effects/action.rs index 641c012cf8..0f162c900d 100644 --- a/shared-libs/crates/start-core/src/service/effects/action.rs +++ b/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, + #[serde(flatten)] + #[ts(skip)] + #[arg(skip)] + event: EventId, #[arg(help = "help.arg.action-id")] action_id: ActionId, #[ts(type = "Record | 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, 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, - /// The `eventId` from the `get-input` whose form this input answers. - #[ts(optional)] - #[arg(long, help = "help.arg.event-id")] - event_id: Option, + #[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, 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,12 +255,10 @@ 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 } } @@ -273,9 +266,9 @@ async fn run_action( #[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, @@ -283,12 +276,13 @@ pub struct CreateTaskParams { 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::::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::(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()); + } +} diff --git a/shared-libs/crates/start-core/src/service/effects/prelude.rs b/shared-libs/crates/start-core/src/service/effects/prelude.rs index efcf2ae872..67ba0be609 100644 --- a/shared-libs/crates/start-core/src/service/effects/prelude.rs +++ b/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, +} +impl EventId { + /// A fresh id for a call made outside any procedure. + pub fn or_new(self) -> Guid { + self.event_id.unwrap_or_default() + } } diff --git a/shared-libs/crates/start-core/src/service/mod.rs b/shared-libs/crates/start-core/src/service/mod.rs index 24c7bc5343..c891a3a92b 100644 --- a/shared-libs/crates/start-core/src/service/mod.rs +++ b/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, recovery_source: Option, init_progress: Option, @@ -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, diff --git a/shared-libs/crates/start-core/src/service/persistent_container.rs b/shared-libs/crates/start-core/src/service/persistent_container.rs index 37471d23b2..49c681006b 100644 --- a/shared-libs/crates/start-core/src/service/persistent_container.rs +++ b/shared-libs/crates/start-core/src/service/persistent_container.rs @@ -366,7 +366,7 @@ impl PersistentContainer { pub async fn init( &self, seed: Weak, - procedure_id: Guid, + event_id: Guid, kind: Option, ) -> Result<(), Error> { let socket_server_context = EffectContext::new(seed); @@ -434,13 +434,7 @@ impl PersistentContainer { } self.rpc_client - .request( - rpc::Init, - InitParams { - id: procedure_id, - kind, - }, - ) + .request(rpc::Init, InitParams { id: event_id, kind }) .await?; self.state.send_modify(|s| s.rt_initialized = true); diff --git a/shared-libs/ts-modules/start-core/lib/Effects.ts b/shared-libs/ts-modules/start-core/lib/Effects.ts index 19ff1633c3..d9c6291d6c 100644 --- a/shared-libs/ts-modules/start-core/lib/Effects.ts +++ b/shared-libs/ts-modules/start-core/lib/Effects.ts @@ -23,7 +23,6 @@ import { Manifest, HostnameInfo, Progress, - Guid, } from './osBindings' import { PackageId, @@ -61,8 +60,6 @@ export type Effects = { run>(options: { packageId?: PackageId actionId: ActionId - /** The `eventId` from the `getInput` whose form this input answers. Required by an action with input. */ - eventId?: Guid input?: Input }): Promise createTask(options: CreateTaskParams): Promise diff --git a/shared-libs/ts-modules/start-core/lib/actions/index.ts b/shared-libs/ts-modules/start-core/lib/actions/index.ts index 21f8e23de6..48a314e56c 100644 --- a/shared-libs/ts-modules/start-core/lib/actions/index.ts +++ b/shared-libs/ts-modules/start-core/lib/actions/index.ts @@ -12,7 +12,8 @@ export type RunActionInput = (form: { /** * Runs an action of this service, or of another one whose `access` admits it. * An action with input opens its form first, so the input is checked against - * the form it answers. + * the form it answers. The target keys that form by the calling procedure's + * event id, so one procedure runs one such action at a time. */ export const runAction = async < Input extends Record, @@ -39,7 +40,6 @@ export const runAction = async < return effects.action.run({ packageId, actionId, - eventId: form.eventId, input: options.input({ spec: form.spec as IST.InputSpec, value: form.value as T.DeepPartial | null, diff --git a/shared-libs/ts-modules/start-core/lib/osBindings/EffectsRunActionParams.ts b/shared-libs/ts-modules/start-core/lib/osBindings/EffectsRunActionParams.ts index e8d31966e9..eb93655ad4 100644 --- a/shared-libs/ts-modules/start-core/lib/osBindings/EffectsRunActionParams.ts +++ b/shared-libs/ts-modules/start-core/lib/osBindings/EffectsRunActionParams.ts @@ -1,14 +1,9 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. import type { ActionId } from './ActionId' -import type { Guid } from './Guid' import type { PackageId } from './PackageId' export type EffectsRunActionParams = { packageId?: PackageId - /** - * The `eventId` from the `get-input` whose form this input answers. - */ - eventId?: Guid actionId: ActionId input: any } diff --git a/shared-libs/ts-modules/start-core/lib/osBindings/EventId.ts b/shared-libs/ts-modules/start-core/lib/osBindings/EventId.ts deleted file mode 100644 index 6431469259..0000000000 --- a/shared-libs/ts-modules/start-core/lib/osBindings/EventId.ts +++ /dev/null @@ -1,4 +0,0 @@ -// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. -import type { Guid } from './Guid' - -export type EventId = { eventId: Guid } diff --git a/shared-libs/ts-modules/start-core/lib/osBindings/index.ts b/shared-libs/ts-modules/start-core/lib/osBindings/index.ts index e08b4913c0..1dba847046 100644 --- a/shared-libs/ts-modules/start-core/lib/osBindings/index.ts +++ b/shared-libs/ts-modules/start-core/lib/osBindings/index.ts @@ -109,7 +109,6 @@ export { EffectsRunActionParams } from './EffectsRunActionParams' export { EncryptedWire } from './EncryptedWire' export { Epp } from './Epp' export { ErrorData } from './ErrorData' -export { EventId } from './EventId' export { ExportActionParams } from './ExportActionParams' export { ExportRangeServiceInterfaceParams } from './ExportRangeServiceInterfaceParams' export { ExportServiceInterfaceParams } from './ExportServiceInterfaceParams' diff --git a/shared-libs/ts-modules/start-core/lib/test/runAction.test.ts b/shared-libs/ts-modules/start-core/lib/test/runAction.test.ts index 1d23245c62..19c9b9aaeb 100644 --- a/shared-libs/ts-modules/start-core/lib/test/runAction.test.ts +++ b/shared-libs/ts-modules/start-core/lib/test/runAction.test.ts @@ -14,27 +14,25 @@ const metadata = { access: 'public' as const, } -/** Routes the effects to one action the way StartOS does: a fresh event id per call unless one is named. */ -function hostedBy(action: ReturnType, caller: string) { - let next = 0 - const effectsFor = (eventId: string) => ({ eventId }) as unknown as Effects +/** Routes a service's effects to one action the way StartOS does: under the calling procedure's event id. */ +function callerEffects( + action: ReturnType, + caller: string, + eventId: string, +) { + const target = { eventId } as unknown as Effects return { - eventId: 'caller-event', + eventId, action: { getInput: jest.fn(async ({ prefill }: { prefill?: unknown }) => action.getInput({ - effects: effectsFor(`event-${next++}`), + effects: target, prefill: (prefill ?? null) as any, caller, }), ), - run: jest.fn( - async ({ eventId, input }: { eventId?: string; input?: any }) => - action.run({ - effects: effectsFor(eventId ?? `event-${next++}`), - input, - caller, - }), + run: jest.fn(async ({ input }: { input?: any }) => + action.run({ effects: target, input, caller }), ), }, } as unknown as Effects @@ -50,7 +48,10 @@ function attachAction(ran: jest.Mock) { address: Value.select({ name: 'Address', default: 'new', - values: { new: 'New', [`${(prefill as any)?.hostId}-0`]: 'Existing' }, + values: { + new: 'New', + [`${(prefill as any)?.hostId}-0`]: 'Existing', + }, }), }), async () => null, @@ -62,9 +63,9 @@ function attachAction(ran: jest.Mock) { } describe('runAction', () => { - test('answers the form it opened, under that form’s event id', async () => { + test('answers the form it opened in the same procedure', async () => { const ran = jest.fn() - const effects = hostedBy(attachAction(ran), 'bitcoind') + const effects = callerEffects(attachAction(ran), 'bitcoind', 'init') await runAction({ effects, @@ -86,25 +87,21 @@ describe('runAction', () => { actionId: 'attach', prefill: { hostId: 'peer' }, }) - expect(effects.action.run).toHaveBeenCalledWith( - expect.objectContaining({ - packageId: 'tor', - actionId: 'attach', - eventId: 'event-0', - }), - ) expect(ran).toHaveBeenCalledWith( { hostId: 'peer', address: 'peer-0' }, 'bitcoind', ) }) - test('an input run under an event id no form was opened for is refused', async () => { + test('a procedure that opened no form cannot run with input', async () => { const ran = jest.fn() - const effects = hostedBy(attachAction(ran), 'bitcoind') + const action = attachAction(ran) + await callerEffects(action, 'bitcoind', 'init').action.getInput({ + actionId: 'attach', + }) await expect( - effects.action.run({ + callerEffects(action, 'bitcoind', 'main').action.run({ actionId: 'attach', input: { hostId: 'peer', address: 'new' }, }),