Skip to content

fix: paginate release lookup and stop dereferencing a null previous release - #10

Merged
goruha merged 3 commits into
cloudposse-github-actions:fix/paginate-release-lookupfrom
johncblandii:fix/paginate-release-lookup
Sep 4, 2026
Merged

fix: paginate release lookup and stop dereferencing a null previous release#10
goruha merged 3 commits into
cloudposse-github-actions:fix/paginate-release-lookupfrom
johncblandii:fix/paginate-release-lookup

Conversation

@johncblandii

Copy link
Copy Markdown
Contributor

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 <tag> 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
  • Baseline reproduces the reported failure — setFailed message and TypeError — on every deep-index case
  • A tag at index 66 of 82 resolves a previous release after the fix
  • With a per-module include_regex, it resolves to a same-module tag
  • The genuine no-previous-release path exits with setFailed's message and no TypeError
  • Default include_regex of .* produces identical output before and after commit 2
  • 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.

The previous-release lookup called `listReleases` without pagination, so only
the 30 most recent releases were scanned. GitHub orders releases by creation
date descending, and a release published from a draft sorts by the date the
draft was created rather than the date it was published, so a release can sit
well beyond the first page. When the current release is not on that page the
loop never sets `currentReleaseFound` and `previousRelease` stays null.

`core.setFailed` does not halt execution, so the null `previousRelease` was
then dereferenced by `compareCommits`, and the resulting
`TypeError: Cannot read properties of null (reading 'tag_name')` masked the
actionable message that had just been emitted.

Paginate with `per_page: 100` and return after `setFailed` so the real message
is the failure.
The previous-release lookup took the first eligible release in the list
regardless of `include_regex`. In a monorepo with per-module tags that is
usually an unrelated module, so `compareCommits` runs over an arbitrary range
and comments on whatever pull requests happen to fall inside it - silently,
with no error.

Concretely, for `dw-cc-ps-posting-service-database@0.1.0` the next release is
skipped by the same-SHA guard and the base becomes
`dw-cc-ps-spring-boot-starter@0.1.0-rc.1`, comparing a database release
against a spring-boot-starter release candidate.

Prefer the first eligible release whose tag also matches `include_regex`. When
no prior release matches, warn and fall back to the previously selected
release, so repos cutting a module's first release - and this action's own
test workflows, whose throwaway prereleases are deleted on teardown - behave
exactly as before. With the default `include_regex` of `.*` this is a no-op.
Copilot AI lite review requested due to automatic review settings September 2, 2026 20:00

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The changes address the reported crash and monorepo base-selection issue with minimal, understandable logic updates and no API/output contract changes.

Pull request overview

This PR fixes a crash and incorrect base-release selection in the composite action’s inline github-script logic by (1) paginating the release lookup and (2) ensuring the action stops after core.setFailed, plus optionally preferring a previous release that also matches include_regex for monorepo tag patterns.

Changes:

  • Paginate repos.listReleases via github.paginate(...) so the current release can be found even when it’s beyond the first page.
  • Return immediately after core.setFailed(...) to avoid dereferencing previousRelease when it is null.
  • Prefer a previous release whose tag_name matches the already-compiled include_regex, with a warning-backed fallback to preserve prior behavior.
File summaries
File Description
action.yml Paginate release enumeration, prevent null dereference after failure, and refine previous-release selection to respect include_regex (with fallback).
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread action.yml Outdated
Comment thread action.yml Outdated
The previous-release lookup assigned to undeclared variables, creating
implicit globals that would break under strict-mode execution. Declare the
variables in this block explicitly.

Limited to the block this change already rewrites; the rest of the script
still relies on implicit globals and is left for the TypeScript rewrite noted
at the top of the file.

@aknysh aknysh left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but @goruha please review

@mergify

mergify Bot commented Sep 3, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@johncblandii

Copy link
Copy Markdown
Contributor Author

The three red checks (test (test-positive.yml), test (test-negative.yml), and the ci gate) are a fork-PR limitation, not a problem with this diff. I can't fix them from my side — flagging what I found in case it's useful.

What happens. The test matrix job in the shared ci.yml uses convictional/trigger-workflow-and-wait to workflow_dispatch the test workflows. It fails at the dispatch step, before action.yml is ever executed:

Triggering workflow:
  workflows/test-positive.yml/dispatches
  {"ref":"fix/paginate-release-lookup","inputs":{...}}
curl: (22) The requested URL returned error: 403
api failed:
  path: workflows/test-positive.yml/dispatches
  response: {
    "message": "Resource not accessible by integration",
    "status": "403"
  }

Why, twice over:

  1. branch.yml asks for permissions: {contents: write, actions: write} and secrets: inherit, but on a pull_request event from a fork GitHub caps the token at read-only and withholds secrets regardless. The job log confirms it: Actions: read, Contents: read, Metadata: read, Secret source: None. workflow_dispatch needs actions: write.
  2. Even with a write token, the dispatch targets ref: fix/paginate-release-lookup in this repo, and that branch only exists on my fork — this repo has just main. It would 404.

ci is only an aggregate gate and fails purely as a consequence: {"lint": "success", "test": "failure", "readme": "skipped"}.

This looks structural rather than new. #10 is the first cross-repository PR this repo has had — #1 through #9 were all branches inside the repo, where the token has write scope and the head ref resolves locally. So I'd expect any fork PR here to go red on these three checks.

What I'd suggest, all of which need write access I don't have:

  • Push this branch into the repo and run CI from there, or re-run the two workflows manually via workflow_dispatch against it. Happy to rebase onto whatever branch name you'd prefer.
  • Or just treat these three as expected-red for fork PRs. lint and context both pass, and the diff touches only action.yml — no workflow files.

For what it's worth, I did reproduce both test workflows' scenarios in the harness described in the PR body, including the same-SHA guard, and both behave identically before and after this change (the test-positive.yml / test-negative.yml rows in that table). That isn't a substitute for really running them, and I'm not claiming otherwise — but it's the closest I can get without write access here.

@goruha
goruha changed the base branch from main to fix/paginate-release-lookup September 4, 2026 12:49
@goruha
goruha merged commit 2b96615 into cloudposse-github-actions:fix/paginate-release-lookup Sep 4, 2026
7 of 10 checks passed
@johncblandii
johncblandii deleted the fix/paginate-release-lookup branch September 4, 2026 13:41
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

These changes were released in v0.0.0-test.include.2b96615.

goruha added a commit that referenced this pull request Sep 4, 2026
…elease (#10) (#11)

## 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 <tag> 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
- [x] `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.

## what
* Describe high-level what changed as a result of these commits (i.e. in
plain-english, what do these changes mean?)
* Use bullet points to be concise and to the point.

## why
* Provide the justifications for the changes (e.g. business case). 
* Describe why these changes were made (e.g. why do these commits fix
the problem?)
* Use bullet points to be concise and to the point.

## references
* Link to any supporting github issues or helpful documentation to add
some context (e.g. stackoverflow).
* Use `closes #123`, if this PR closes a GitHub issue `#123`

Co-authored-by: John C. Bland II <johncblandii@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants