Infinite loop of skipped execution

[smklein]

Heya, I'm using "skip duplicate actions" in a private GitHub repo as shown below:

on: [ push, pull_request ]

jobs:                                                                                                                  
  skip_duplicate_jobs:                                                                                                 
    runs-on: ubuntu-18.04                                                                                              
    outputs:                                                                                                     
      should_skip: ${{ steps.skip_check.outputs.should_skip }}                                                   
    steps:                                              
      - id: skip_check                                                                                           
        # fkirc/[email protected] (this is the current master branch)
        uses: fkirc/skip-duplicate-actions@867de26591aa7542d65bc38f576c5a244c3c6034                                    
        with:                                                                                                    
          # For workflows which are triggered concurrently with the same                                               
          # contents, attempt to execute them exactly once.                                                      
          concurrent_skipping: 'same_content_newer'                                                              
  check-style:                                                                                                         
    needs: skip_duplicate_jobs                                                                                   
    if: ${{ needs.skip_duplicate_jobs.outputs.should_skip != 'true' }}   
    # ... The rest of the "check-style" job.
 # Other jobs follow the format of "check-style" -- check "skip_duplicate_jobs" first, then run.

Ideally, this would avoid re-running jobs for which the contents of the repo are file-for-file identical. However, for a commit introducing new changes to any files, it should execute the job.

In reality, I'm seeing that execution for many of the jobs is skipped, with the output:
Skip execution because the exact same files are concurrently checked in older <link to older run Foo>

Clicking on that older run, it also appears to be skipped with the output:
Skip execution because the exact same files are concurrently checked in older <link to older run Bar>

... Continuing to click through, I see:
Skip execution because the exact same files are concurrently checked in older <link to older run Foo>

(the loop is that Foo points to Bar which points to Foo - they both skip running jobs, because they blame the other).

I think this is because the jobs are executed on both "push" and "pull_request", and the timing somehow means that both jobs think the other has executed checks, when in reality no one has.

I think the checks in the current code, as shown below:

skip-duplicate-actions/src/index.ts

Lines 233 to 247 in 867de26

	} else if (context.concurrentSkipping === "same_content" || context.concurrentSkipping === "same_content_newer") {
	const concurrentDuplicate = concurrentRuns.find((run) => run.treeHash === context.currentRun.treeHash);
	if (concurrentDuplicate) {
	if (context.concurrentSkipping === "same_content") {
	core.info(`Skip execution because the exact same files are concurrently checked in ${concurrentDuplicate.html_url}`);
	exitSuccess({ shouldSkip: true });
	} else if (context.concurrentSkipping === "same_content_newer") {
	const concurrentIsOlder = concurrentRuns.find((run) => run.runNumber < context.currentRun.runNumber);
	if (concurrentIsOlder) {
	core.info(`Skip execution because the exact same files are concurrently checked in older ${concurrentDuplicate.html_url}`);
	exitSuccess({ shouldSkip: true });
	}
	}
	}
	}

This code calculates the following:

• concurrentDuplicate is any completed run with the same treeHash value.
• concurrentIsOlder is any completed run with a lower run number.

So the condition (I believe) I'm hitting is:

1. context.concurrentSkipping === "same_content_newer"
2. Some run with the same treehash value exists
3. Some run with a lower run number exists (NOT necessarily the same one in step 2)

However, I think this logic is wrong - seems like we should search for a run with the the tree hash equal and a lower run number in the same call to concurrentRuns.find, rather than in two separate calls. I believe the second check (that some run exists with a lower run number) is not particularly useful without this caveat.

[smklein]

Fixed by #125