Improve typings and README

README.md

@@ -125,7 +125,7 @@ The planner the agent uses can also be invoked directly, this is useful for test
 
 ```typescript
 import { Planner } from 'mahler/planner';
-import { plan, stringify } from 'mahler/testing';
+import { sequence, stringify } from 'mahler/testing';
 
 // Create a new planner
 const planner = Planner.from({ tasks: [plusOne] });
@@ -153,16 +153,18 @@ const plusOne = Task.from<number>({
 	condition: (state, { target }) => state < target,
 	effect: (state) => ++state._,
 	action: async (state) => {
-		// The counter at runtime will only get updated
-		// if storeCounter succeeds
-		state._ = await storeCounter(state._ + 1);
+		// storeCounter stores the given value on disk or
+		// throws if an error happens
+		await storeCounter(++state._);
 	},
 	description: '+1',
 });
 ```
 
 The `action` property above updates the state and stores the value. While the `action` and `effect` functions are similar, the `action` defines what will actually happen when the task is chosen as part of a plan, while the `effect` just provides a "simulation" of the changes on the system state to be used during planning.
 
+One more thing to note is that if `storeCounter` throws while writing to disk, the agent runtime will revert the internal state to before the `plusOne` action was executed to prevent the system state from becoming inconsistent.
+
 Let's update the task to also read the stored state before updating it to avoid writing an inconsistent state.
 
 ```typescript
@@ -178,7 +180,7 @@ const plusOne = Task.from({
 
 		// We only update the stored value if it is below the target
 		if (state._ < target) {
-			state._ = await storeCounter(state._ + 1);
+			await storeCounter(++state._);
 		}
 	},
 });
@@ -269,7 +271,7 @@ const store = Task.from<System>({
 	},
 	action: async (state) => {
 		// We write the counter and toggle the flag
-		state._.counter === (await storeCounter(state._.counter));
+		await storeCounter(state._.counter);
 		state._.needsWrite = false;
 	},
 	description: 'storeCounter',
@@ -389,15 +391,15 @@ const plusTwo = Task.from<number>({
 	// state and the target is bigger than one
 	condition: (state, { target }) => target - state > 1,
 	// Defining the method property makes this task into a method
-	// A method should not modify the state, just return a sequence of applicable actions
+	// A method should never modify the state, just return a sequence of applicable actions
 	method: (_, { target }) => [plusOne({ target }), plusOne({ target })],
 	description: '+2',
 });
 ```
 
 Now there is a lot happening here. We have replaced `effect` and `action` in the task constructor with `method`. We are also directly using the `plusOne` task as a function and passing the target as one of the properties of the argument object. Let's parse this example piece by piece.
 
-The code above is the way to create composite tasks in Mahler, called methods. Using `method` tells the task constructor to return a `MethodTask` instead of an `ActionTask` (like `plusOne`).
+The code above is the way to create compound tasks in Mahler, called methods. Using `method` tells the task constructor to return a `MethodTask` instead of an `ActionTask` (like `plusOne`).
 
 A method should not directly modify the state, but return instead a sequence of actions that are applicable under the given conditions. As we see above, in this case, the method returns a sequence of two `plusOne` tasks applied to the target.
 
@@ -407,13 +409,17 @@ Objects generated by task constructors are callable, and receive part of the `Co
 // Another test utility
 import { runTask } from 'mahler/testing';
 
-// This executes the task. Useful for testing
-// `runTask` calls the task with the given state and context
+// We can execute the task directly by binding it to a context
+// and then providing a state
+console.log(await plusOne({ target: 3 })(0)); // 1
+
+// Mahler provides the `runTask` helper function to
+// call the task with the given state and context
 // the call will throw if the task condition fails
 console.log(await runTask(plusOne, 0, { target: 3 })); // 1
 
-// `runTask` in this case expands the method into its actions and
-// applies the actions sequentially
+// `runTask` also works with methods, expanding the method
+// into its actions and executing the actions sequentially
 console.log(await runTask(doPlusTwo, 0, { target: 3 })); // 2
 ```
 
@@ -520,9 +526,6 @@ const plusThree = Task.from<number>({
 });
 ```
 
-> [!NOTE]
-> Methods cannot call themselves recursively or as part of call chain. This is forbidden to prevent infinite recursion. Using recursion means the method will effectively never be called.
-
 ## Lenses
 
 Let's say now that we want our agent to manage multiple counters. Let's redefine our system state once more to do this
@@ -574,7 +577,7 @@ Note also that instead of the usual `Task.from` we preface the `from` call with
 Now we can go a bit deeper on the context object, we mentioned before that it contains the context from the task selection process during planning. More specifically, the context object is composed by
 
 - A `target` property, with the expected target value for the part of the state that the task acts on. This will have the same type as the first argument of the function, which will match the type pointed by the `lens` property. We'll see when we talk about [operations](#operations) that this property is not present for delete or wildcard (`*`) operation.
-- A `system` property, providing a copy of the global state at the moment the task is used. This can be used, for instance, to define tasks with conditions on parts of the system that are unrelated to the sub-object pointed to by the lens.
+- A `system` property, providing a read-only copy of the global state at the moment the task is used. This can be used, for instance, to define tasks with conditions on parts of the system that are unrelated to the sub-object pointed to by the lens.
 - A `path` property, indicating the full path of the lens where the task is being applied.
 - Zero or more properties with the names of the placeholders in the lens definition. For instance is the lens is `/a/:aId/b/:bId/c/:cId`, `aId`, `bId`, and `cId` will be provided in the context, with the right type for the property within the path.
 
@@ -915,8 +918,12 @@ graph TD
 What about deletion? What if you want to search for the following plan?
 
 ```typescript
+import { UNDEFINED } from 'mahler';
+
 const res = planner.findPlan(
 	{ counters: { a: 0, b: 1 } },
+	// `UNDEFINED` is a symbol that tells mahler to
+	// look for a target that doesn't have the property `b`
 	{ counters: { a: 2, b: UNDEFINED } },
 );
 ```
@@ -948,6 +955,8 @@ In some cases, we may want to create tasks that are applicable to any operation,
 const delCounter = Task.of<System>().from({
 	op: '*',
 	lens: '/counters/:counterId',
+	// Do not use the task if the counter was already deleted
+	condition: (counter) => counter != null,
 	effect: (counter) => {
 		// we need to manually delete the counter here as the library cannot infer this step for wildcard tasks
 		counter.delete();
@@ -958,6 +967,9 @@ const delCounter = Task.of<System>().from({
 
 The `View` type provides the convenience `delete()` method for such scenario.
 
+> [!NOTE]
+> As wildcard tasks are applicable to any operation, restricting the condition is very important to prevent the planner from repeatedly chosing the task.
+
 ### Operation precedence
 
 What if for a given operation we have multiple tasks that are applicable? For instance, for an `update` operation on path `/counters/a`, we could have a task that applies to the path `/counters` (as with the parallel update example) and a task that applies to a given counter (like `plusOne`). In that case, the planner will try to apply the task that applies to highest level path first (`/counters` in this case), only if that fails, it will try the task that is applicable to `/counters/a`.
@@ -1009,7 +1021,7 @@ For `delete` operations, the planner will expand the search to sub-elements of t
 ];
 ```
 
-This is because of the implied hierarchy in the state, deleting a value of type `/a/b` may need to make sure that no sub-elements of type `c` exist before the delete can take place. This is similar to a database, where deletes of related entities required requires cascading deletes.
+This is because of the implied hierarchy in the state, deleting a value of type `/a/b` may need to make sure that no sub-elements of type `c` exist before the delete can take place. This is similar to a database, where deletes of related entities require cascading deletes.
 
 The planner doesn't enforce any hierarchy though, so it is up to the developer to add conditions to tasks to let the planner know if certain delete sequence is necessary.
 
@@ -1041,7 +1053,7 @@ const mySensor = MySystem.sensor({
 
 ## Sensors
 
-We are almost done introducing concepts, before continuing however, let's move to a better example. Let's write an agent for a simple Space Heater controller. The heater design is very simple, it is composed by a resistor that can be turned ON or OFF to heat the room, and a termometer that detects the room temperature. The heater interface allows to set a target room temperature. The controller will turn the resistor ON if the temperature is below target or OFF if the temperature is above target.
+We are almost done introducing concepts, before continuing however, let's move to a better example. Let's write an agent for a simple space heater controller. The heater design is very simple, it is composed by a resistor that can be turned ON or OFF to heat the room, and a termometer that detects the room temperature. The heater interface allows to set a target room temperature. The controller will turn the resistor ON if the temperature is below target or OFF if the temperature is above target.
 
 Let's start first by modelling the state. As the per the hardware design, the state needs to keep track of the resistor state and the room temperature.
 

lib/task/props.ts

@@ -12,6 +12,12 @@ export type ContextWithSystem<
 	TOp extends AnyOp = Update,
 > = Context<TState, TPath, TOp> & { system: TState };
 
+type ReadOnlyContextWithSystem<
+	TState = unknown,
+	TPath extends PathType = Root,
+	TOp extends AnyOp = Update,
+> = ReadOnly<Context<TState, TPath, TOp> & { system: TState }>;
+
 /**
  * A descriptor for this task. The descriptor can be either a string or a Context
  * instance. The description does not receive the current state to allow actions to
@@ -29,7 +35,7 @@ type ConditionFn<
 	TOp extends AnyOp = Update,
 > = (
 	s: TOp extends Create ? never : ReadOnly<Lens<TState, TPath>>,
-	c: ReadOnly<ContextWithSystem<TState, TPath, TOp>>,
+	c: ReadOnlyContextWithSystem<TState, TPath, TOp>,
 ) => boolean;
 
 type EffectFn<
@@ -38,7 +44,7 @@ type EffectFn<
 	TOp extends AnyOp = Update,
 > = (
 	view: View<TState, TPath, TOp>,
-	ctx: ContextWithSystem<TState, TPath, TOp>,
+	ctx: ReadOnlyContextWithSystem<TState, TPath, TOp>,
 ) => void;
 
 type ActionFn<
@@ -47,7 +53,7 @@ type ActionFn<
 	TOp extends AnyOp = Update,
 > = (
 	view: View<TState, TPath, TOp>,
-	ctx: ContextWithSystem<TState, TPath, TOp>,
+	ctx: ReadOnlyContextWithSystem<TState, TPath, TOp>,
 ) => Promise<void>;
 
 type MethodFn<
@@ -56,7 +62,7 @@ type MethodFn<
 	TOp extends AnyOp = Update,
 > = (
 	s: TOp extends Create ? never : ReadOnly<Lens<TState, TPath>>,
-	c: ReadOnly<ContextWithSystem<TState, TPath, TOp>>,
+	ctx: ReadOnlyContextWithSystem<TState, TPath, TOp>,
 ) => Instruction<TState> | Array<Instruction<TState>>;
 
 /**

tests/composer/tasks.ts

@@ -36,15 +36,12 @@ export const fetchImage = App.task({
 	},
 	action: async (image, { imageName }) => {
 		await new Promise((resolve, reject) =>
-			docker
-				.pull(imageName)
-				.catch(reject)
-				.then((stream) => {
-					stream.on('data', () => void 0);
-					stream.on('error', reject);
-					stream.on('close', resolve);
-					stream.on('finish', resolve);
-				}),
+			docker.pull(imageName).then((stream) => {
+				stream.on('data', () => void 0);
+				stream.on('error', reject);
+				stream.on('close', resolve);
+				stream.on('finish', resolve);
+			}),
 		);
 
 		// Get the image using the name

tests/orchestrator/planner.ts

@@ -10,7 +10,7 @@ import {
 	migrateService,
 	startService,
 	stopService,
-	removeService,
+	uninstallService,
 	removeRelease,
 	removeApp,
 } from './tasks';
@@ -20,15 +20,11 @@ export const planner = Planner.from<Device>({
 		fetch,
 		createApp,
 		createRelease,
-		// NOTE: right now we need to make sure to put `migrateService` before
-		// `installService` in the task list, otherwise the planner will always chose to
-		// recreate services even if a migration suffices. This is because the planner
-		// just returns the first path it finds instead of the shortest
 		migrateService,
 		installService,
 		startService,
 		stopService,
-		removeService,
+		uninstallService,
 		removeRelease,
 		removeApp,
 	],

tests/orchestrator/planning.spec.ts

@@ -404,7 +404,10 @@ describe('orchestrator/planning', () => {
 			plan()
 				.action("initialize release 'r1' for app 'a0'")
 				.action("pull image 'alpine:latest' with tag 'a0_main:r1'")
-				.action("migrate unchanged service 'main' of app 'a0 to release 'r1' '")
+				.action("migrate unchanged service 'main' of app 'a0 to release 'r1'")
+				.action(
+					"remove metadata for service 'main' from release 'r0' of app 'a0'",
+				)
 				.action("remove release 'r0'")
 				.action("pull image 'alpine:latest' with tag 'a0_other:r1'")
 				.action(

tests/orchestrator/tasks.ts

@@ -312,14 +312,29 @@ export const startService = Task.of<Device>().from({
 });
 
 /**
- * Rename a service between releases if the image and service configuration has not changed
+ * Delete service data from a release
  *
- * Condition: the service exists (it has a container id), the container is not running, the container configuration
- *            matches the target configuration, and source and target images are the same
- * Effect: move the service from the source release to the target release
+ * This is used to migrate a service between releases. It is only to be used within a method
+ */
+const deleteService = Task.of<Device>().from({
+	op: 'delete',
+	lens: '/apps/:appUuid/releases/:releaseUuid/services/:serviceName',
+	effect: () => {
+		/* the operation already tells the framework to delete afterwards */
+	},
+	description: ({ serviceName, appUuid, releaseUuid }) =>
+		`remove metadata for service '${serviceName}' from release '${releaseUuid}' of app '${appUuid}'`,
+});
+
+/**
+ * Rename a service container between releases if the image and service configuration has not changed
+ *
+ * This is called only as part of the migrateService method so we skip the condition.
+ *
+ * Effect: copy the service from the source release to the target release
  * Action: rename the container using the docker API
  */
-export const migrateService = Task.of<Device>().from({
+const renameServiceContainer = Task.of<Device>().from({
 	op: 'create',
 	lens: '/apps/:appUuid/releases/:releaseUuid/services/:serviceName',
 	condition: (
@@ -342,9 +357,6 @@ export const migrateService = Task.of<Device>().from({
 		const currRelease = Object.keys(releases).find((u) => u !== releaseUuid)!;
 		const currService = releases[currRelease]?.services[serviceName];
 
-		// Remove the release from the current release
-		delete releases[currRelease].services[serviceName];
-
 		// Move the service to the new release
 		service._ = currService;
 	},
@@ -356,18 +368,59 @@ export const migrateService = Task.of<Device>().from({
 		const currRelease = Object.keys(releases).find((u) => u !== releaseUuid)!;
 
 		const currService = releases[currRelease]?.services[serviceName];
-		delete releases[currRelease].services[serviceName];
 
 		// Rename the container
 		await docker.getContainer(currService.containerId!).rename({
 			name: getContainerName({ releaseUuid, serviceName }),
 		});
 
-		// Move the container to the new release
+		// Copy the container to the new release
 		service._ = currService;
 	},
 	description: (ctx) =>
-		`migrate unchanged service '${ctx.serviceName}' of app '${ctx.appUuid} to release '${ctx.releaseUuid}' '`,
+		`migrate unchanged service '${ctx.serviceName}' of app '${ctx.appUuid} to release '${ctx.releaseUuid}'`,
+});
+
+/**
+ * Rename a service container between releases if the image and service configuration has not changed
+ *
+ * This is a method as changes on two different releases need to be performed.
+ * The method will first rename the container and link the service on the target release. It then will
+ * remove the service metadata from the current release
+ *
+ * Condition: the service exists (it has a container id), the container configuration
+ *            matches the target configuration, and source and target images are the same
+ */
+export const migrateService = Task.of<Device>().from({
+	op: 'create',
+	lens: '/apps/:appUuid/releases/:releaseUuid/services/:serviceName',
+	expansion: 'sequential',
+	condition: (
+		_,
+		{ appUuid, releaseUuid, serviceName, system: device, target },
+	) => {
+		const { releases } = device.apps[appUuid];
+		const [currentRelease] = Object.keys(releases).filter(
+			(u) => u !== releaseUuid,
+		);
+		const currService = releases[currentRelease]?.services[serviceName];
+		return (
+			currService != null &&
+			isEqualConfig(currService, target) &&
+			target.image === currService.image
+		);
+	},
+	method: (
+		_,
+		{ system: device, appUuid, serviceName, releaseUuid, target },
+	) => {
+		const { releases } = device.apps[appUuid];
+		const currRelease = Object.keys(releases).find((u) => u !== releaseUuid)!;
+		return [
+			renameServiceContainer({ target, appUuid, serviceName, releaseUuid }),
+			deleteService({ appUuid, releaseUuid: currRelease, serviceName }),
+		];
+	},
 });
 
 /**
@@ -436,7 +489,7 @@ export const stopService = Task.of<Device>().from({
  * Effect: remove the service from the device state
  * Action: remove the container using the docker API
  */
-export const removeService = Task.of<Device>().from({
+export const uninstallService = Task.of<Device>().from({
 	op: '*',
 	lens: '/apps/:appUuid/releases/:releaseUuid/services/:serviceName',
 	condition: (service) =>