From f3c0f29ba56c30063011db2e2f7e31e22dd2e0be Mon Sep 17 00:00:00 2001 From: Bastien Orivel Date: Thu, 9 Apr 2026 12:36:07 +0200 Subject: [PATCH] Don't return the root of a CoT twice when it's also a link Creating the CoT here, with `no_check_task` being default (False) https://github.com/mozilla-releng/scriptworker/blob/6b0b66b03e9ec07b462a550b19b742a863d6b17a/src/scriptworker/cot/verify.py#L2089-L2091 Then because `check_task` is true, it calls `build_link` with itself, adding a `LinkOfTrust` with the same task ID. https://github.com/mozilla-releng/scriptworker/blob/6b0b66b03e9ec07b462a550b19b742a863d6b17a/src/scriptworker/cot/verify.py#L2044-L2045 Then we call `verify_task_types` on that chain, which calls `get_all_links_in_chain` and actually verifies links. That's where the log is coming from. https://github.com/mozilla-releng/scriptworker/blob/6b0b66b03e9ec07b462a550b19b742a863d6b17a/src/scriptworker/cot/verify.py#L1806-L1813 The problem is that `get_all_links_in_chain` returns `[self] + self.links` if the `CoT` object isn't a decision task https://github.com/mozilla-releng/scriptworker/blob/6b0b66b03e9ec07b462a550b19b742a863d6b17a/src/scriptworker/cot/verify.py#L212 So it ends up returning the root task twice, one because of `[self]` and once because `check_task` added the root as a `LoT` to the `CoT`. --- `get_all_links_in_chain` was added in 0356bbb4b8e73c9af4d8a2ca7eec4e88cc0b97a6 but at the time, `check_task` didn't exist so it was correct. The bug was introduced by 8b6ab79d2fa40a61148b50da28c3b907ce4a776c which started adding the root of a `CoT` as a link, making `get_all_links_in_chain` return "dupes" for it. This most likely never got caught because it only happens when check_task is True which isn't the case for scriptworkers, only `verify_cot` and only when checking a non decision task. This fixes the issue by not relying on the task type to know whether or not it should return itself in the chain but by looking down the graph to see if it's already included or not. --- src/scriptworker/cot/verify.py | 2 +- tests/test_cot_verify.py | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/scriptworker/cot/verify.py b/src/scriptworker/cot/verify.py index 58821d1c..92a23954 100644 --- a/src/scriptworker/cot/verify.py +++ b/src/scriptworker/cot/verify.py @@ -222,7 +222,7 @@ def get_all_links_in_chain(self): list: of all ``LinkOfTrust``s to verify. """ - if self.is_decision() and self.get_link(self.task_id): + if self.task_id in self.dependent_task_ids(): return self.links return [self] + self.links diff --git a/tests/test_cot_verify.py b/tests/test_cot_verify.py index 20434fb4..043f8af9 100644 --- a/tests/test_cot_verify.py +++ b/tests/test_cot_verify.py @@ -362,6 +362,12 @@ async def test_get_all_links_in_chain(chain, decision_link, build_link): chain.links = [build_link, decision_link] assert set(chain.get_all_links_in_chain()) == set([decision_link, build_link]) + # Ensure we don't return the root twice when `check_task` is True for non decision tasks + chain.task_type = "signing" + self_link = cotverify.LinkOfTrust(chain.context, chain.name, chain.task_id) + chain.links = [build_link, decision_link, self_link] + assert set(chain.get_all_links_in_chain()) == set([build_link, decision_link, self_link]) + # is_try {{{1 @pytest.mark.asyncio