Don't return the root of a CoT twice when it's also a link#787
Merged
Eijebong merged 1 commit intomozilla-releng:mainfrom Apr 9, 2026
Merged
Don't return the root of a CoT twice when it's also a link#787Eijebong merged 1 commit intomozilla-releng:mainfrom
Eijebong merged 1 commit intomozilla-releng:mainfrom
Conversation
bhearsum
approved these changes
Apr 9, 2026
Contributor
bhearsum
left a comment
There was a problem hiding this comment.
Thanks for the sleuthing and the fix!
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 0356bbb but at the time, `check_task` didn't exist so it was correct. The bug was introduced by 8b6ab79 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.
1ecf564 to
f3c0f29
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Creating the CoT here, with
no_check_taskbeing default (False)scriptworker/src/scriptworker/cot/verify.py
Lines 2089 to 2091 in 6b0b66b
Then because
check_taskis true, it callsbuild_linkwith itself, adding aLinkOfTrustwith the same task ID.scriptworker/src/scriptworker/cot/verify.py
Lines 2044 to 2045 in 6b0b66b
Then we call
verify_task_typeson that chain, which callsget_all_links_in_chainand actually verifies links. That's where the log is coming from.scriptworker/src/scriptworker/cot/verify.py
Lines 1806 to 1813 in 6b0b66b
The problem is that
get_all_links_in_chainreturns[self] + self.linksif theCoTobject isn't a decision taskscriptworker/src/scriptworker/cot/verify.py
Line 212 in 6b0b66b
So it ends up returning the root task twice, one because of
[self]and once becausecheck_taskadded the root as aLoTto theCoT.get_all_links_in_chainwas added in0356bbb but at the time,
check_taskdidn't exist so it was correct. The bug was introduced by 8b6ab79 which started adding the root of aCoTas a link, makingget_all_links_in_chainreturn "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_cotand 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.