feat: expands workflow engine to support service commands by rina23q · Pull Request #4338 · thin-edge/thin-edge.io

rina23q · 2026-09-14T10:14:43Z

Proposed changes

This is part 2 of the service-commands feature. Part 1 was #4308.

This PR does:

expands tedge-agent's workflow engine to also support type = "service".
The agent now reacts to the commands of the services of its own device.
ships service_restart.toml, the workflow of the restart action for services.
declares restart for thin-edge's own services when they start.
documents the MQTT API of the service commands

To try out the full end-to-end feature, use PR #4299.

Types of changes

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
Documentation Update (if none of the other choices apply)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue

#4299: Initial service command feature end-to-end PR
#4242: Issue
#4308: Part 1: tedge service CLI

Checklist

I have read the CONTRIBUTING doc
I have signed the CLA (in all commits with git commit -s. You can activate automatic signing by running just prepare-dev once)
I ran just format as mentioned in CODING_GUIDELINES
I used just check as mentioned in CODING_GUIDELINES
I have added tests that prove my fix is effective or that my feature works
I have added necessary documentation (if appropriate)

Further comments

rina23q · 2026-09-14T10:16:08Z

I'm planning to rename this file after PR got approvals to reduce noises in this PR.

codecov · 2026-09-14T10:26:15Z

Codecov Report

❌ Patch coverage is 79.97257% with 146 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...ore/tedge_agent/src/operation_workflows/persist.rs	27.77%	64 Missing and 1 partial ⚠️
crates/core/tedge_api/src/workflow/supervisor.rs	91.15%	10 Missing and 3 partials ⚠️
...common/tedge_config/src/tedge_toml/tedge_config.rs	0.00%	10 Missing ⚠️
crates/core/tedge_agent/src/agent.rs	0.00%	10 Missing ⚠️
.../core/tedge_agent/src/operation_workflows/actor.rs	90.42%	7 Missing and 2 partials ⚠️
...ent/src/operation_workflows/entity_store_client.rs	95.74%	3 Missing and 5 partials ⚠️
...agent/src/operation_workflows/shipped_workflows.rs	0.00%	8 Missing ⚠️
crates/core/tedge_mapper/src/core/mapper.rs	0.00%	7 Missing ⚠️
crates/core/tedge_mapper/src/lib.rs	0.00%	7 Missing ⚠️
crates/core/tedge_api/src/service_command.rs	50.00%	6 Missing ⚠️
... and 2 more

📢 Thoughts on this report? Let us know!

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions · 2026-09-14T10:59:36Z

Robot Results

✅ Passed	❌ Failed	⏭️ Skipped	Total	Pass %	⏱️ Duration
1027	0	10	1027	100	3h38m17.023245s

rina23q · 2026-09-16T13:00:50Z

+                if entity_type == EntityType::Service {
+                    return Ok(vec![]);
+                }


This change actually reverts #3747.

The problem is here.
Previously, te/device/main/service/app1/restart capability message is a trigger to declare the c8y_Restart supported operation in c8y. (same for log_upload => c8y_LogfileUpload, and more built-in operations).

However, in the goal of the service command feature, te/device/main/service/app1/restart should be interpreted as a supported action of the service. For c8y, the operation name will be c8y_ServiceCommand, and restart is a member of c8y_SupportedServiceCommands.

Therefore, here disables to convert service capability messages. It will be revisited in the part 3 PR.

jarhodes314 · 2026-09-18T09:12:53Z

+
+        let own_device = &self.device_topic_id;
+        // Here `get` uses a HTTP request behind so that agent running on child device can reach the entity store
+        match self.entity_store.get(target).await {


When I got claude to have a look through the PR, it noted that this is a risk - specifically making an HTTP request in the operations loop could block the loop if the HTTP request is encounters errors and this could significantly slow the operations handling.

After considering this for a bit, my thoughts are that if the entity store HTTP server is having problems, we won't be able to accept new operations anyway, so slow operation handling isn't necessarily a huge issue. I do slightly wonder how problematic this could prove though if you sent an operation to a bunch of child devices.

I put 10s timeout to the request to avoid blocking the loop forever. Also, If agent is running on a child device, HTTP is the only way to query the entity data to the entity store.

I do slightly wonder how problematic this could prove though if you sent an operation to a bunch of child devices.

Yeah, maybe true...

jarhodes314 · 2026-09-18T11:52:08Z

 }

+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub struct ServiceActions {


This API could do with some documentation. It's not obvious to me what it actually does/what it's intended to communicate to whom. I'm wondering whether it's more of a naming issue, but because I don't really understand the API, I don't really know.

I can understand what you said.

ServiceActions is the set of capability messages the service publishes about itself at startup:

declared: a retained {} on te/<service-topic-id>/cmd/<action>

withdrawn: a retained empty message on the same topic

withdrawn is needed only for one case, a mapper that ran standalone before and now runs inside tedge run all:

inside tedge run all the mapper has no init system unit of its own, so it cannot support restart

the restart capability of the standalone run stays on the broker as a retained message, so the mapper has to clear it

Agreed that the naming is bad. Small history: they are based on what was written in OpenSpec, but I manually renamed them in the reference guide because I didn't like them. Then, the names lost the connection to this struct.

So my proposal what I will do:

rename the type to ActionCapabilities { declared, cleared }, with declaring() and clearing(), matching the "declaring an action" and "clearing an action" wording of the service commands reference page

document on the type and why clearing is needed

Done in d703e73

jarhodes314 · 2026-09-18T12:10:16Z

 }

+#[tokio::test]
+async fn a_command_for_a_service_of_this_device_moves_to_executing() -> Result<(), DynError> {


Suggested change

async fn a_command_for_a_service_of_this_device_moves_to_executing() -> Result<(), DynError> {

async fn an_init_command_for_a_registered_service_of_this_device_moves_to_executing() -> Result<(), DynError> {

Applied in 6fc6f07

jarhodes314 · 2026-09-18T12:11:49Z

+        )
+        .with_parent("device/main//".parse().unwrap())]),
+    )
+    .await?;


Generally, I really dislike returning Result<...> from tests, because unlike when calling unwrap() or expect() within the test, we don't get a backtraces showing us what want wrong in the test when it fails.

Removed Result<...> in 6fc6f07 and 0bb1b30. Note that I didn't change the existing tests which return Result<...>.

albinsuresh · 2026-09-22T12:46:41Z

+### Declaring an action
+
+A service declares an action by publishing a retained empty JSON object `{}`
+on `te/<service-topic-id>/cmd/<action>`, one topic per action.


I wouldn't use the action term at this MQTT topic level, as cmd/<cmd-type> is already a well established convention in the MQTT APIs for devices. I suspect that the action term leaked from the tedge service <action> command. Since these commands directly map to workflows where the keyword action has a different meaning, it'd just create additional confusion. So, at this level I'd just call it cmd-type itself as that's what the MQTT API calls it for any entity, be it device or service.

Fixed in 43c51e3

albinsuresh · 2026-09-22T12:53:51Z

+    "serviceName": "nodered",
+    "serviceType": "service"


It doesn't feel right to re-specify the serviceName and serviceType in the command payload, when they are already registered as part of the service registration. Purely from an agent/API standpoint, why should an external user/client care what the name of the service is, when he already knows the topic id?

I understand that this came from an existing limitation where the workflow engine doesn't have access to the entity info, and hence this was a workaround. But. I'd be in favour of fixing that gap. Let the agent inject the entity metadata into the payload when it moves the command from init to executing. And let the workflow actions refer to it using paths like ${.entity.parent}, ${.entity.id}, ${.entity.twin.name} etc.

I think having ${.entity.*} is useful in workflows. For this case, we will need ${entity.twin.type} (= type field in JSON) and ${entity.twin.name} (= name field in JSON)

To do so, we will need not only agent change but also HTTP API change:

We have to open up HTTP API to return also twin_data. For child device agent, HTTP API is the only source to query the entity data. Currently GET /te/v1/entities/<topic-id> doesn't return name and type, which is required here.

Do you know why our HTTP entity API doesn't return twin_data?

The twin data needs to be queried using thhe GET /te/v1/entities/{topic-id}/twin/{fragment-key} endpoint documented here

I found "re-specify" is half correct.

Since name and type fields are not mandatory, even if we provide the new placeholder ${.entity.*), they cannot be resolved when they are missing in the entity store.

Only if they are already given in the registration message or twin message, then indeed this is "re-specify".

So, what I suggest here are:

introduce ${.entity.*}, simply it's useful

serviceName and serviceType in the payload are optional. Because of the case above where type or name is missing in the entity store, c8y converter will put serviceName and serviceType from the operation payload.

serviceName / serviceType wins over ${.entity.twin.name} / ${.entity.twin.type}, defined in toml level. Maybe this kind of literal ${.payload.serviceType ?? .entity.twin.type ?? "service"} will be helpful, but extra work.

${.entity.*} implementation is ready in my local branch, but since it doesn't solve the initial purpose, I think we should do after the third PR, not in this PR.

albinsuresh · 2026-09-22T13:06:01Z

@@ -0,0 +1,16 @@
+operation = "probe"


When custom service commands will be routed through the workflows, we will end up duplicating service definitions as follows:

operation = "new-command-type" ... [execute] script = "sudo -n tedge service ${.topic.operation} ${.payload.serviceName} --type ${.payload.serviceType}"

where the body remains the same and only the operation key changes.

The impact of this on a user is that whenever he wants to add support for a new command for his service, he needs to write a plugin (which actually captures the service specific logic) and then duplicate a workflow definition like this by simply duplicating the template as well. That duplication just feels redundant. So, how about introducing a "wildcard workflow definition" support where a single workflow definition can be reused for multiple operation types, with the option to selectively override them as well as we'll have to do with the service_restart workflow. Something like this:

# /etc/tedge/operations/service_command.toml type = "service" operation = "*" ... [execute] script = "sudo -n tedge service ${.topic.operation} ${.payload.serviceName} --type ${.payload.serviceType}" timeout_second = 180 on_exit.0 = "successful" on_exit.2 = { status = "failed", reason = "This action is not supported for that type of service" } ...

where the operation = "*" qualifies it as the wildcard template.

Useful, but I would not do this in this PR. Should be follow-up. This will affect the existing device workflows as well, and the service command feature can work without this.

albinsuresh · 2026-09-22T13:12:42Z

+
+        let own_device = &self.device_topic_id;
+        // Here `get` uses a HTTP request behind so that agent running on child device can reach the entity store
+        match self.entity_store.get(target).await {


This came as quite a surprise to me, where the workflow engine has to query the entity store via the HTTP interface when the same agent owns both. But I realised that child-devices were the primary motivation for this design, as an agent running on a child device isn't aware of its own child services without relying on the entity store hosted by the main device. That made me wonder if it makes sense for the agent on the child device to keep track of its own services. It's not an entity store, but just having knowledge about its own children, which didn't seem unreasonable. But yeah, we need to think this through as this is a minimal projection of the entity store itself and we need to keep them consistent. So, need not be done in this PR, but something to think about.

This came as quite a surprise to me, where the workflow engine has to query the entity store via the HTTP interface when the same agent owns both.

Yeah, actually this is the choice to simplify the query part that can be applicable for both main and child. However, maybe it was too lazy. I'm now considering to use entity store directly for the main device without using HTTP.

As for children, as you said, currently we must use HTTP to query the entity from agent running on child device.

Now as long as agent is running on the main device, it directly access to the entity store, without HTTP. ef0c257

albinsuresh

Happy to approve the PR after the 2 issues raised in code comments are resolved.

The following limitations can be filled in follow-up PRs as I don't feel these should block the delivery of the core feature itself despite those limitations:

Since the same workflow actor in the agent is driving the workflows for operations meant for itself and the services of the same device, everything is sequential, meaning a long running operation on one service can block the scheduled operations on another service or the device itself, while both should have ideally executed parallelly.
Eliminating the need to provide serviceName and serviceType in the init commands

albinsuresh · 2026-10-07T16:33:35Z

        runtime.spawn(twin_manager_builder).await?;
        runtime.spawn(downloader_actor_builder).await?;
        runtime.spawn(uploader_actor_builder).await?;
+        runtime.spawn(http_actor).await?;


Now that this actor is used only by the agent on child devies and not by the main device, shouldn't this also be spawned conditionally only on child devices?

Good catch. Changed in 075a5b4

albinsuresh · 2026-10-07T16:39:30Z

            return Ok(());
        };
+        let Some(entity_type) = self.command_target_type(&target, &state).await else {
+            return Ok(());


Instead of silently ignoring an entity store lookup failure in the command_target_type call, shouldn't we fail the operation itself? Or ideally retry?

Good point. The lookup is now retried, and it does not block the actor. e72deef

The agent cannot fail the command on a lookup failure: it subscribes to the commands of every entity, so it does not know whether the command is its own or belongs to the agent of another device. Failing it would race with that agent.

So the lookup runs in a background task, with 9 retries from 1s to 256s (about 10 minutes in total). If all retries fail, the agent logs an error and the command stays in init. If the command is cleared while the lookup runs, it is not executed.

Bravo555 · 2026-10-08T12:59:04Z

+Only the workflow for type device declares a capability
+    ${capabilities}=    Execute Command    tedge mqtt sub te/+/+/+/+/cmd/probe --retained-only --duration 2s
+    Should Contain    ${capabilities}    [te/device/main///cmd/probe] {}
+    Should Not Contain    ${capabilities}    te/device/main/service/sleeper/cmd/probe
+


question: I'm not sure what's the reason for this behaviour.

In A command targeting a service runs a service workflow this workflow is run, why do we assert that the capability for it isn't published? AFAIK there's nothing in the spec/docs mentioning it, but the task 1.3b mentions:

1.3b Filter capability_messages on the entity type, so a service workflow does not declare a capability on the device's topics

but still I'm not sure of the reason. I'd assume it means "service commands shouldn't declare device capability" but currently it means "service commands shouldn't declare any capabilities". Where is my mistake?

EDIT: Also, I understand that currently tedge-mapper-c8y does not convert service capabilities messages, which is expected and will be revisited, but is a separate thing from service capabilities messages not appearing in the first place.

Also changes in mapper-publishing-agent-supported-ops.robot are related to this.

Your reading is right: a service workflow declares no capability at all.
The reason why workflows for type = "service" don't publish capabilities is that the existence of a service workflow does not tell which services support its action. For example, if a service workflow for probe command exists, we cannot know which service really supports it. Also, in the early ideation phase, we decided to use "push-model", in which a service capability must be declared by the service entity. (though this can be changed to "pull-model" in the later phase).

I will add one sentence in operation-workflow.md. => Done in 29295d7.

A workflow with `type = "service"` publishes no capability. Each service [declares its own actions](../service-commands#declaring-an-action).

Also changes in mapper-publishing-agent-supported-ops.robot are related to this.

Reason is explained in this thread: #4338 (comment). Now all ../cmd/<cmd_name> for service entities are considered as "service actions", so ../cmd/restart for services should not be converted to c8y_Restart supported operation as it is for devices.

albinsuresh · 2026-10-09T07:19:41Z

+            info!("Ignoring the command addressed to {target}: no such entity is registered");
+            None
+        }
+        Err(err) => {
+            error!("Not executing the command addressed to {target}, as its registration data cannot be read: {err}");
+            None


For these two cases, I'm still not fully sure if ignoring the command in the original init state without failing it is the right thing to do. But I also understand that we have taken similar stances in the past where the agent wouldn't react to anything that it doesn't clearly isn't its responsibility, assuming that some other external service might be responsible for handling them.

Signed-off-by: Rina Fujino <rina.fujino.23@gmail.com>

Drive a restart of a service of the main device, of tedge-agent itself, and of a service of a child device. Check that a command picks the workflow typed for its target, when a device workflow and a service workflow share an operation name. The agent of the parent device is now restarted by the setup of the child device suite, so that its HTTP server binds the address the child reads the entity store from. The agent of the child device is started by the setup too, so that each test runs on its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Rina Fujino <rina.fujino.23@gmail.com>

…eration A command declared on a service topic is a service command, and the operations Cumulocity defines act on a device. The mapper registered `c8y_Restart`, `c8y_LogfileRequest` and the others on the managed object of the service. The system test suite covering the supported operations of a service is skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Rina Fujino <rina.fujino.23@gmail.com>

Signed-off-by: Rina Fujino <rina.fujino.23@gmail.com>

rina23q requested review from a team, albinsuresh, didier-wenzek, jarhodes314 and reubenmiller as code owners September 14, 2026 10:14

rina23q deployed to Test Pull Request September 14, 2026 10:14 — with GitHub Actions Active

rina23q commented Sep 14, 2026

View reviewed changes

rina23q changed the title ~~Feat/agent service commands~~ feat: expands workflow engine to support service commands Sep 14, 2026

rina23q had a problem deploying to Test Auto September 14, 2026 10:28 — with GitHub Actions Failure

reubenmiller added skip-release-notes Don't include the ticket in the auto generated release notes theme:monitoring Theme: Service monitoring and watchdogs labels Sep 15, 2026

rina23q deployed to Test Pull Request September 16, 2026 11:14 — with GitHub Actions Active

rina23q had a problem deploying to Test Auto September 16, 2026 11:25 — with GitHub Actions Failure

rina23q commented Sep 16, 2026

View reviewed changes

rina23q deployed to Test Pull Request September 16, 2026 15:49 — with GitHub Actions Active

rina23q had a problem deploying to Test Auto September 16, 2026 16:17 — with GitHub Actions Failure

rina23q force-pushed the feat/agent-service-commands branch from 842005c to 738de55 Compare September 17, 2026 14:46

rina23q deployed to Test Pull Request September 17, 2026 14:46 — with GitHub Actions Active

rina23q deployed to Test Auto September 17, 2026 14:56 — with GitHub Actions Active

jarhodes314 reviewed Sep 18, 2026

View reviewed changes

albinsuresh reviewed Sep 22, 2026

View reviewed changes

rina23q deployed to Test Pull Request September 24, 2026 01:05 — with GitHub Actions Active

rina23q deployed to Test Auto September 24, 2026 01:16 — with GitHub Actions Active

rina23q deployed to Test Pull Request September 24, 2026 18:15 — with GitHub Actions Active

rina23q deployed to Test Auto September 24, 2026 18:25 — with GitHub Actions Active

albinsuresh reviewed Oct 7, 2026

View reviewed changes

Bravo555 self-requested a review October 8, 2026 07:52

Bravo555 self-assigned this Oct 8, 2026

rina23q deployed to Test Pull Request October 8, 2026 10:54 — with GitHub Actions Active

rina23q deployed to Test Auto October 8, 2026 11:17 — with GitHub Actions Active

Bravo555 reviewed Oct 8, 2026

View reviewed changes

Bravo555 removed their assignment Oct 8, 2026

rina23q requested a deployment to Test Pull Request October 8, 2026 14:07 — with GitHub Actions Waiting

albinsuresh approved these changes Oct 9, 2026

View reviewed changes

rina23q and others added 8 commits October 9, 2026 11:03

spec: the cloud-agnostic interface of service commands

0865b4b

Signed-off-by: Rina Fujino <rina.fujino.23@gmail.com>

feat(tedge_api): scope workflow definitions by entity type

ec93cf6

Signed-off-by: Rina Fujino <rina.fujino.23@gmail.com>

feat(tedge_agent): drive the commands of the services of its own device

ee4ab69

Signed-off-by: Rina Fujino <rina.fujino.23@gmail.com>

feat(tedge_api): thin-edge's own services declare only their restart

d6c6bb4

Signed-off-by: Rina Fujino <rina.fujino.23@gmail.com>

docs: document the service commands of the agent

db6bc58

Signed-off-by: Rina Fujino <rina.fujino.23@gmail.com>

test: wait for tedge-agent before registering entities

5536b80

Signed-off-by: Rina Fujino <rina.fujino.23@gmail.com>

rina23q force-pushed the feat/agent-service-commands branch from 29295d7 to 5536b80 Compare October 9, 2026 11:07

rina23q deployed to Test Pull Request October 9, 2026 11:07 — with GitHub Actions Active

rina23q deployed to Test Auto October 9, 2026 11:18 — with GitHub Actions Active

rina23q added this pull request to the merge queue Oct 9, 2026

Merged via the queue into thin-edge:main with commit 7580885 Oct 9, 2026
36 checks passed

This was referenced Oct 12, 2026

feat: maps service commands from Cumulocity #4366

Open

Support Cumulocity's service commands #4242

Open

	async fn a_command_for_a_service_of_this_device_moves_to_executing() -> Result<(), DynError> {
	async fn an_init_command_for_a_registered_service_of_this_device_moves_to_executing() -> Result<(), DynError> {

Conversation

rina23q commented Sep 14, 2026

Proposed changes

Types of changes

Paste Link to the issue

Checklist

Further comments

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Sep 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented Sep 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Robot Results

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rina23q Sep 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rina23q Sep 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rina23q Sep 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

albinsuresh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rina23q Oct 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Sep 14, 2026 •

edited

Loading

github-actions Bot commented Sep 14, 2026 •

edited

Loading

rina23q Sep 24, 2026 •

edited

Loading

rina23q Sep 24, 2026 •

edited

Loading

rina23q Sep 24, 2026 •

edited

Loading

rina23q Oct 8, 2026 •

edited

Loading

Bravo555 Oct 8, 2026 •

edited

Loading

rina23q Oct 8, 2026 •

edited

Loading