Cache activity before opening retirement PRs

.github/workflows/retire.yml

@@ -10,10 +10,17 @@ on:
 
 permissions: {}
 
+# Runs read the activity cache artifact of the previous run, so they must not overlap.
+concurrency:
+  group: retire
+
 jobs:
   retire:
     name: retire
     runs-on: ubuntu-latest
+    permissions:
+      # To download the activity cache artifact from the previous run (.csv)
+      actions: read
     steps:
       - uses: actions/create-github-app-token@v2
         id: app-token
@@ -36,8 +43,26 @@ jobs:
           token: ${{ steps.app-token.outputs.token }}
           # Fetch full history to determine when committers were added
           fetch-depth: 0
+      - name: Restore activity cache
+        env:
+          GH_TOKEN: ${{ github.token }}
+        run: |
+          runId=$(gh run list --workflow=retire.yml --status=success --limit 1 --json databaseId --jq '.[0].databaseId // empty')
+          if [[ -n "$runId" ]] && gh run download "$runId" --name retire-cache --dir .cache; then
+            echo "Restored activity cache from run $runId"
+          else
+            echo "No previous activity cache, starting fresh"
+          fi
       - name: Run script
-        run: scripts/retire.sh NixOS nixpkgs nixpkgs-committers members "yesterday 1 month ago" "1 year ago"
+        run: scripts/retire.sh NixOS nixpkgs nixpkgs-committers members "yesterday 1 month ago" "1 year ago" "7 days ago"
         env:
           GH_TOKEN: ${{ steps.app-token.outputs.token }}
           PROD: "1"
+          CACHE_FILE: .cache/retire-cache.csv
+      - name: Save activity cache
+        uses: actions/upload-artifact@v4
+        with:
+          name: retire-cache
+          path: .cache/retire-cache.csv
+          if-no-files-found: error
+          overwrite: true

.gitignore

@@ -1 +1,2 @@
 .tmp
+.cache

scripts/README.md

@@ -1,8 +1,7 @@
 # Nixpkgs Committer management script testing
 
 The recommended way to test these scripts is to run them in a GitHub test organisation with the right setup.
-Since creating your own takes some time, you can just ask @infinisil to get added to his test setup instead,
-whose identifiers will be used here.
+Since creating your own takes some time, you can just ask @infinisil to get added to his test setup instead, whose identifiers will be used here.
 
 ## Setup
 
@@ -102,51 +101,69 @@ git push -f -u origin HEAD
 
 ### Test sequence
 
-The following sequence tests all code paths:
+The following sequence tests all code paths.
+
+The `CONFIRM_CUTOFF` argument is passed as `now` so that a single run is enough to open a retirement PR.
+In CI it is set to `7 days ago` instead, which together with the `CACHE_FILE` activity cache (persisted between runs as a workflow artifact) requires a week of daily runs to agree before a PR is opened.
 
 1. Run the script with the `active` repo argument to simulate CI running without inactive users:
    ```bash
-   scripts/retire.sh infinisil-test-org active nixpkgs-committers members-test 'yesterday 1 month ago' now
+   scripts/retire.sh infinisil-test-org active nixpkgs-committers members-test 'yesterday 1 month ago' now now
    ```
 
    Check that no PR would be opened.
+1. Run the previous command again with `CACHE_FILE` set:
+   ```bash
+   CACHE_FILE=.tmp-cache.csv scripts/retire.sh infinisil-test-org active nixpkgs-committers members-test 'yesterday 1 month ago' now now
+   CACHE_FILE=.tmp-cache.csv scripts/retire.sh infinisil-test-org active nixpkgs-committers members-test 'yesterday 1 month ago' now now
+   rm .tmp-cache.csv
+   ```
+
+   Check that the first run writes your activity to the cache file and that the second run skips the `/commits` query for your user because of it.
 1. Run the script with the `empty` repo argument to simulate CI running with inactive users:
 
    ```bash
-   scripts/retire.sh infinisil-test-org empty nixpkgs-committers members-test 'yesterday 1 month ago' now
+   scripts/retire.sh infinisil-test-org empty nixpkgs-committers members-test 'yesterday 1 month ago' now now
+   ```
+
+   Check that it would only create a PR for your own user and not the "new-committer-1" or "new-committer-2" user.
+   Also check that with a `CONFIRM_CUTOFF` in the past (as used in CI), no PR would be opened yet:
+
+   ```bash
+   scripts/retire.sh infinisil-test-org empty nixpkgs-committers members-test 'yesterday 1 month ago' now '7 days ago'
    ```
 
-   Check that it would only create a PR for your own user and not the "new-committer-1" or "new-committer-2" user before running it again with `PROD=1` to actually do it:
+   Then run it again with `PROD=1` to actually do it:
 
    ```bash
-   PROD=1 scripts/retire.sh infinisil-test-org empty nixpkgs-committers members-test 'yesterday 1 month ago' now
+   PROD=1 scripts/retire.sh infinisil-test-org empty nixpkgs-committers members-test 'yesterday 1 month ago' now now
    ```
 
    Check that it created the PR appropriately, including assigning the "retirement" label.
    You can undo this step by closing the PR.
 1. Run it again to simulate CI running again later:
    ```bash
-   PROD=1 scripts/retire.sh infinisil-test-org empty nixpkgs-committers members-test 'yesterday 1 month ago' now
+   PROD=1 scripts/retire.sh infinisil-test-org empty nixpkgs-committers members-test 'yesterday 1 month ago' now now
    ```
    Check that no other PR is opened.
 1. Run it again with `now` as the notice cutoff date to simulate the time interval passing:
    ```bash
-   PROD=1 scripts/retire.sh infinisil-test-org empty nixpkgs-committers members-test now now
+   PROD=1 scripts/retire.sh infinisil-test-org empty nixpkgs-committers members-test now now now
    ```
    Check that it undrafted the previous PR and posted an appropriate comment.
 1. Run it again to simulate CI running again later:
    ```bash
-   PROD=1 scripts/retire.sh infinisil-test-org empty nixpkgs-committers members-test now now
+   PROD=1 scripts/retire.sh infinisil-test-org empty nixpkgs-committers members-test now now now
    ```
    Check that no other PR is opened.
 1. Reset by marking the PR as a draft again, then run it again with the `active` repo argument to simulate activity during the time interval:
    ```bash
-   PROD=1 scripts/retire.sh infinisil-test-org active nixpkgs-committers members-test now now
+   PROD=1 scripts/retire.sh infinisil-test-org active nixpkgs-committers members-test now now now
    ```
    Check that it gets undrafted with a comment listing the new activity.
 1. Close the PR, then run the script again with no activity and for an earlier close cutoff, simulating that the retirement was delayed:
    ```bash
-   PROD=1 scripts/retire.sh infinisil-test-org empty nixpkgs-committers members-test now '1 day ago'
+   PROD=1 scripts/retire.sh infinisil-test-org empty nixpkgs-committers members-test now '1 day ago' now
    ```
 
    Check that no other PR is opened.

scripts/retire.sh

@@ -5,7 +5,9 @@ source "$(dirname -- "${BASH_SOURCE[0]}")"/common.sh
 shopt -s nullglob
 
 usage() {
-  log "Usage: $0 ORG ACTIVITY_REPO MEMBER_REPO DIR NOTICE_CUTOFF CLOSE_CUTOFF"
+  log "Usage: $0 ORG ACTIVITY_REPO MEMBER_REPO DIR NOTICE_CUTOFF CLOSE_CUTOFF CONFIRM_CUTOFF"
+  log ""
+  log "Optionally set CACHE_FILE to a path for persisting activity observations between runs."
   exit 1
 }
 
@@ -15,14 +17,30 @@ MEMBER_REPO=${3:-$(usage)}
 DIR=${4:-$(usage)}
 NOTICE_CUTOFF=${5:-$(usage)}
 CLOSE_CUTOFF=${6:-$(usage)}
+CONFIRM_CUTOFF=${7:-$(usage)}
 
 mainBranch=$(git branch --show-current)
 noticeCutoff=$(date --date="$NOTICE_CUTOFF" +%s)
 
+nowEpoch=$(date +%s)
+
 # People that received the commit bit after this date won't be retired
 newCutoff=$(date --date="1 year ago" +%s)
 # Users whose retirement PRs were closed after this date won't be retired
 closeCutoff=$(date --date="$CLOSE_CUTOFF" +%s)
+# Users first observed as inactive after this date won't be retired yet;
+# they keep being checked until the observation is old enough.
+# This prevents a single spurious empty response from the /commits endpoint
+# from causing an accidental retirement PR (see issue #100).
+confirmCutoff=$(date --date="$CONFIRM_CUTOFF" +%s)
+
+# People are considered active if they merged a PR after this date
+yearAgo=$(date --date='1 year ago' +%s)
+# Cached merges newer than this are trusted without querying GitHub again.
+# The gap until the 1-year mark means the /commits endpoint gets queried
+# for about a month of daily runs before a retirement PR can be opened,
+# so a retirement is backed by many independent queries agreeing.
+queryCutoff=$(date --date='11 months ago' +%s)
 
 # We need to know when people received their commit bit to avoid retiring them within the first year.
 # For now this is done either with the git creation date of the file, or its contents:
@@ -54,6 +72,33 @@ else
   trap 'rm -rf "$tmp"' exit
 fi
 
+# Activity observed by previous runs, so that the unreliable /commits
+# endpoint doesn't need to be trusted on a single response (see issue #100).
+# The cache only holds positive evidence (merges the API actually returned)
+# plus the time somebody was first observed as inactive,
+# so a spurious empty response can never make a cached-active user look inactive.
+# Bump this version to invalidate all previously cached data.
+cacheVersion=1
+declare -A cachedMergeEpoch cachedMergePr cachedFirstInactive
+if [[ -n "${CACHE_FILE:-}" ]]; then
+  CACHE_FILE=$(realpath -m -- "$CACHE_FILE")
+  if [[ -f "$CACHE_FILE" ]] && [[ $(head -n 1 "$CACHE_FILE") == "# retire-cache v$cacheVersion" ]]; then
+    while IFS=, read -r cLogin cMergeEpoch cMergePr cFirstInactive; do
+      [[ "$cLogin" == '#'* ]] && continue
+      cachedMergeEpoch[$cLogin]=$cMergeEpoch
+      cachedMergePr[$cLogin]=$cMergePr
+      cachedFirstInactive[$cLogin]=$cFirstInactive
+    done < "$CACHE_FILE"
+    log "Loaded activity cache with ${#cachedMergeEpoch[@]} entries from $CACHE_FILE"
+  else
+    log "No usable activity cache at $CACHE_FILE, starting fresh"
+  fi
+fi
+
+# Matches the merge lines produced by the jq filter below,
+# capturing the merge date and PR number for the cache
+mergeLineRegex='^- `([^`]+)` .*(#[0-9]+)$'
+
 mkdir -p "$DIR"
 cd "$DIR"
 for login in *; do
@@ -108,22 +153,72 @@ for login in *; do
     continue
   fi
 
-  PR_PREFIX="$ORG/$ACTIVITY_REPO" \
-  trace gh api -X GET /repos/"$ORG"/"$ACTIVITY_REPO"/commits \
-    -f since="$(date --date='1 year ago' --iso-8601=seconds)" \
-    -f author="$login" \
-    -f committer=web-flow \
-    -f per_page=100 \
-    --jq '.[] |
-      # PR merge commits have two parents. We also check it’s an
-      # authentic GitHub commit, because… why not?
-      select((.parents | length) == 2 and .commit.verification.verified) |
-      (.commit.message | capture(" \\((?<pr>#[0-9]+)\\)$").pr) as $pr |
-      "- `\(.commit.committer.date)` – \(env.PR_PREFIX)\($pr)"' \
-    > "$tmp/$login"
-  mergeCount=$(wc -l <"$tmp/$login")
+  # What previous runs observed about this person's merge activity
+  mergeEpoch=${cachedMergeEpoch[$login]:-0}
+  mergePr=${cachedMergePr[$login]:-}
+  firstInactive=${cachedFirstInactive[$login]:-0}
+
+  : > "$tmp/$login"
+  mergeCount=0
+  queried=
+  if [[ "$prState" != open ]] && (( mergeEpoch > queryCutoff )); then
+    # Recent cached activity already rules out retirement,
+    # no need to bother the unreliable /commits endpoint
+    :
+  else
+    queried=1
+    if [[ "$prState" == open ]]; then
+      # The PR comment lists activity from the whole past year
+      sinceEpoch=$yearAgo
+    else
+      # Otherwise only merges newer than the cached one are of interest
+      sinceEpoch=$(( mergeEpoch > yearAgo ? mergeEpoch + 1 : yearAgo ))
+    fi
+    PR_PREFIX="$ORG/$ACTIVITY_REPO" \
+    trace gh api -X GET /repos/"$ORG"/"$ACTIVITY_REPO"/commits \
+      -f since="$(date --date=@"$sinceEpoch" --iso-8601=seconds)" \
+      -f author="$login" \
+      -f committer=web-flow \
+      -f per_page=100 \
+      --jq '.[] |
+        # PR merge commits have two parents. We also check it’s an
+        # authentic GitHub commit, because… why not?
+        select((.parents | length) == 2 and .commit.verification.verified) |
+        (.commit.message | capture(" \\((?<pr>#[0-9]+)\\)$").pr) as $pr |
+        "- `\(.commit.committer.date)` – \(env.PR_PREFIX)\($pr)"' \
+      > "$tmp/$login"
+    mergeCount=$(wc -l <"$tmp/$login")
+  fi
+
+  if (( mergeCount > 0 )); then
+    # Cache the newest merge (first line, the endpoint returns newest first)
+    if [[ $(head -n 1 "$tmp/$login") =~ $mergeLineRegex ]]; then
+      newestMergeEpoch=$(date --date="${BASH_REMATCH[1]}" +%s)
+      if (( newestMergeEpoch > mergeEpoch )); then
+        mergeEpoch=$newestMergeEpoch
+        mergePr=${BASH_REMATCH[2]}
+      fi
+    fi
+    firstInactive=0
+  elif [[ -n "$queried" ]] && (( firstInactive == 0 )); then
+    # An empty response might just be a timeout of the endpoint (issue #100),
+    # so this only starts the inactivity clock; retirement additionally needs
+    # all queries until the confirmation cutoff to keep coming back empty
+    firstInactive=$nowEpoch
+  fi
+
+  # Remember the observations for the cache written at the end of the run
+  cachedMergeEpoch[$login]=$mergeEpoch
+  cachedMergePr[$login]=$mergePr
+  cachedFirstInactive[$login]=$firstInactive
 
   if [[ "$prState" == open ]]; then
+    if (( mergeCount == 0 && mergeEpoch > yearAgo )); then
+      # The query came back empty even though the cache proves activity
+      # (issue #100), so reconstruct the activity list from the cache
+      echo "- \`$(date --date=@"$mergeEpoch" --iso-8601=seconds)\` – $ORG/$ACTIVITY_REPO$mergePr" > "$tmp/$login"
+      mergeCount=1
+    fi
     # If there is an open PR already
     prNumber=$(jq .number <<< "$prInfo")
     epochCreatedAt=$(jq '.created_at | fromdateiso8601' <<< "$prInfo")
@@ -156,7 +251,15 @@ for login in *; do
     else
       log "$login has a retirement PR pending"
     fi
-  elif (( mergeCount <= 0 )); then
+  elif (( mergeEpoch > yearAgo )); then
+    if (( mergeCount > 0 )); then
+      log "$login is active with $mergeCount new merges"
+    else
+      log "$login is active, last known merge $mergePr on $(date --date=@"$mergeEpoch" --iso-8601=seconds) (cached)"
+    fi
+  elif (( firstInactive > confirmCutoff )); then
+    log "$login appears inactive since $(date --date=@"$firstInactive" --iso-8601=seconds), continuing to check before opening a PR"
+  else
     log "$login has become inactive, opening a PR"
     # If there is no PR yet, but they have become inactive
     (
@@ -189,8 +292,18 @@ for login in *; do
         /repos/"$ORG"/"$MEMBER_REPO"/issues/"$prNumber"/labels \
         -f "labels[]=retirement" >/dev/null
     )
-  else
-    log "$login is active with $mergeCount merges"
   fi
   log ""
 done
+
+if [[ -n "${CACHE_FILE:-}" ]]; then
+  mkdir -p "$(dirname -- "$CACHE_FILE")"
+  {
+    echo "# retire-cache v$cacheVersion"
+    for login in *; do
+      echo "$login,${cachedMergeEpoch[$login]:-0},${cachedMergePr[$login]:-},${cachedFirstInactive[$login]:-0}"
+    done
+  } > "$CACHE_FILE".tmp
+  mv "$CACHE_FILE".tmp "$CACHE_FILE"
+  log "Wrote activity cache to $CACHE_FILE"
+fi