From 2b966155e1ff626a9ba3e9f9b45f9f04bb37eece Mon Sep 17 00:00:00 2001 From: "John C. Bland II" Date: Fri, 4 Sep 2026 07:49:19 -0500 Subject: [PATCH] fix: paginate release lookup and stop dereferencing a null previous release (#10) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Fixes a deterministic crash on any release that is not on the first page of `listReleases` — the action dies with `TypeError: Cannot read properties of null (reading 'tag_name')`, which buries the accurate error message it emits one line earlier. - Fixes previous-release selection ignoring `include_regex`, which in a monorepo with per-module tags silently comments on pull requests from an unrelated module. Two reproductions in `DriveWealth/dw-cc-posting-service` (private, 82 releases): | run | tag | position in `listReleases` | | --- | --- | --- | | `33548407262` | `dw-cc-posting-service-database@0.1.0` | index **66** of 82 | | `32513890189` | `dw-cc-ps-posting-service-database@0.1.0` | index **42** of 82 | Both fail identically. Run `32513890189` was `run_attempt: 2` — the re-run failed the same way, so this is not a transient API issue: ``` This action requires that at least one release prior to exists, regardless of "include_regex". Unhandled error: TypeError: Cannot read properties of null (reading 'tag_name') ``` ## Changes Two commits, deliberately separable. ### 1. `listReleases` pagination + `return` after `setFailed` `github.rest.repos.listReleases` was called without pagination, so it returned at most 30 releases. GitHub orders releases by **creation** date descending, and a release published from a draft keeps the draft's creation date — so a draft cut weeks ago and published today lands deep in the list rather than at the top. When the current release is not on that first page, the loop never sets `currentReleaseFound` and `previousRelease` stays `null`. `core.setFailed` does not halt execution, so `compareCommits({ base: previousRelease.tag_name })` then ran against `null`. The resulting `TypeError` is what surfaces in the Actions UI, masking the actionable message. - Paginate via `github.paginate(github.rest.repos.listReleases, { …, per_page: 100 })`. `paginate` returns the array directly, so the loop iterates `releases` rather than `releases.data`. - `return` after `core.setFailed(...)` so the real message is the failure. ### 2. Prefer a previous release that also matches `include_regex` **Separable — feel free to drop this commit; commit 1 stands alone.** Previous-release selection ignored `include_regex`, so in a monorepo the base is usually an unrelated module. Traced for `dw-cc-ps-posting-service-database@0.1.0`: index 43 (`dw-cc-ps-margin-data@0.1.0`) is skipped by the existing same-SHA guard, so the base became index 44, `dw-cc-ps-spring-boot-starter@0.1.0-rc.1`. The action then compares a database release against a spring-boot-starter RC and comments on whatever pull requests fall in that arbitrary range — wrong PRs, silently, with no error. This commit prefers the first eligible release whose tag also matches the already-compiled `include_regex` pattern. The existing same-SHA guard (the "test scenarios" comment block) is untouched and still evaluated first. Backward compatibility: - With the default `include_regex` of `.*` this is a **no-op** — every tag matches. - When `include_regex` matches no prior release at all, it emits a `core.warning` and falls back to exactly the release that would have been chosen before. That keeps working for a repo cutting a module's first release — and, importantly, for this repo's own `test-positive.yml`, whose `v0.0.0-test.include.*` prereleases are deleted by the teardown job with `--cleanup-tag`. At run time no prior release matches that workflow's `include_regex`, so without the fallback the positive test would start failing. ## Testing There is no JS test harness in this repo, so I extracted the inline `script:` block from `action.yml` and executed it against stubbed `github` / `core` / `context` objects with an 82-release fixture mirroring the ordering above. The `listReleases` stub deliberately returns only the first page, so any un-paginated call still reproduces the bug. Assertions are on the `base` argument that actually reaches `compareCommits`. Run against `v0.2.0` (`e823005`), then after each commit: | case | v0.2.0 | + commit 1 | + commit 2 | | --- | --- | --- | --- | | tag at index 66, per-module regex | `TypeError` | `dw-cc-ps-events@0.15.0` | `dw-cc-posting-service-database@0.0.9` | | tag at index 42, regex `.*` | `TypeError` | `dw-cc-ps-spring-boot-starter@0.1.0-rc.1` | same | | tag at index 42, per-module regex (same-module predecessor exists) | `TypeError` | `dw-cc-ps-spring-boot-starter@0.1.0-rc.1` | `dw-cc-ps-posting-service-database@0.0.5` | | per-module regex, **no** same-module predecessor | `TypeError` | `…spring-boot-starter@0.1.0-rc.1` | same, plus fallback warning | | oldest release, genuinely no predecessor | `setFailed` **plus `TypeError`** | `setFailed`, no throw | `setFailed`, no throw | | single-module repo, tag inside first page | `dw-cc-ps-ledger@0.76.0` | same | same | | `test-positive.yml` scenario | `0.2.0` | `0.2.0` | `0.2.0`, plus fallback warning | | `test-negative.yml` scenario | `{"comments":[]}` | same | same | - [x] Baseline reproduces the reported failure — `setFailed` message **and** `TypeError` — on every deep-index case - [x] A tag at index 66 of 82 resolves a previous release after the fix - [x] With a per-module `include_regex`, it resolves to a same-module tag - [x] The genuine no-previous-release path exits with `setFailed`'s message and no `TypeError` - [x] Default `include_regex` of `.*` produces identical output before and after commit 2 - [x] Both `test-positive.yml` and `test-negative.yml` scenarios reproduce unchanged, including the same-SHA guard - [ ] `test-positive.yml` / `test-negative.yml` actually executed — **not done.** They are `workflow_dispatch`-only and need repo secrets plus write access to this repo, which I don't have. The last two rows above are the harness reproducing their scenarios, not real workflow runs. I did not add a workflow test for the >30-releases path: covering it in `test-positive.yml` would mean creating 30+ throwaway releases in this repo on every run. Happy to add a Node-based regression test for the extracted script if you'd want that as a separate change. ## Notes - No breaking changes. No input or output signature changes, so `README.md` / `README.yaml` are untouched. `atmos.yaml` and the workflows are untouched. - No TypeScript rewrite, per the note at the top of `action.yml` — the diff is deliberately minimal and this stays a `0.x.x` change. - **Unrelated observation, not fixed here:** the `switch (context.eventName)` block has the same missing-`return` pattern. The `workflow_dispatch`-without-`tag` case and the `default:` case both call `core.setFailed` and then fall through into `getRelease({ release_id: context.payload.release.id })`, throwing a second, more confusing `TypeError`. Left alone to keep this PR scoped — glad to follow up if you want it. --- action.yml | 41 ++++++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/action.yml b/action.yml index f608e77..03da9da 100644 --- a/action.yml +++ b/action.yml @@ -120,33 +120,56 @@ runs: } // Find a previous release in order to find commits and later use them to find associated pull requests. - releases = await github.rest.repos.listReleases({ + // This must be paginated: releases are returned newest-first by creation date (not publication date), + // so a release published from a long-lived draft can sit well beyond the first page. + const releases = await github.paginate(github.rest.repos.listReleases, { owner: context.repo.owner, - repo: context.repo.repo, + repo: context.repo.repo, + per_page: 100, }); - previousRelease = null; - currentReleaseFound = false; + let previousRelease = null; + let previousReleaseFallback = null; + let currentReleaseFound = false; console.log(`Looking for a release prior to ${currentTag}...`); - for (release of releases.data) { + for (const release of releases) { if (currentReleaseFound && (release.target_commitish != currentRelease.target_commitish || !release.target_commitish.match(/[0-9a-f]{40}/))) { // In test scenarios where a positive test scenario release and a negative test scenario are created at from the same target_commitish, // we want to avoid setting this release as the previous (base) release, as no commits will be found when comparing the two. // However, we obviously want to account for normal scenarios where the target_commitish of two releases is the same and is not a SHA, // for example two releases with target_commmitish as 'main'. This is seemingly the best way to infer this given the limited information // returned by the listReleases endpoint. - previousRelease = release; - break; + if (previousReleaseFallback == null) { + previousReleaseFallback = release; + } + // Prefer a previous release whose tag also matches 'include_regex'. In a monorepo with per-module + // tags the immediately preceding release usually belongs to an unrelated module, which would make + // the comparison range - and therefore the pull requests commented on - arbitrary. + if (release.tag_name.match(pattern)) { + previousRelease = release; + break; + } } else if (release.tag_name == currentTag) { currentReleaseFound = true; } } + if (previousRelease == null && previousReleaseFallback != null) { + // Preserve the previous behaviour when 'include_regex' matches no prior release at all, so that repos + // releasing a module for the first time (and this action's own test workflows) keep working. + core.warning(`No release prior to ${currentTag} matches the supplied 'include_regex'. Falling back to "${previousReleaseFallback.tag_name}".`); + previousRelease = previousReleaseFallback; + } + if (previousRelease == null) { + // 'core.setFailed' does not halt execution, so return explicitly. Otherwise the 'compareCommits' call + // below dereferences a null 'previousRelease' and the resulting TypeError masks the message above. core.setFailed(`This action requires that at least one release prior to ${currentTag} exists, regardless of "include_regex".`); - } else { - console.log(`Found a prior release to "${currentTag}": "${previousRelease.tag_name}"`); + return { + "comments": [] + }; } + console.log(`Found a prior release to "${currentTag}": "${previousRelease.tag_name}"`); // Compare commits, as a pre-requisuite for finding associated pull requests. commitsResponse = await github.rest.repos.compareCommits({