ci: replace the PR-title bump script with release-please - #86

[Must Fix] The integration-gcp-lb job is gone and nothing replaces it.

The old ci.yml had five jobs; this file has four. Makefile:43-44 still defines test-integration-gcp-lb and spec/integration/gcp_lb_keepalive_spec.rb still exists, but grep -rn -i gcp .github/ at this revision returns nothing, so no workflow runs it any more. The ci environment's STREAM_GCP_API_SECRET, STREAM_GCP_API_KEY and STREAM_GCP_BASE_URL become unreferenced at the same time.

Why it matters: that spec is the regression guard for the bug this repo shipped as 11.0.1, pooled TLS connections outliving the GCP SSL-proxy idle window and failing the next request with SSL_read: unexpected eof. It is now a test that exists and never runs, which is worse than not having it.

Fix: add an integration-gcp-lb job here mirroring the deleted one, or delete the make target, the spec and the three ci environment values together and say so in the PR body.

Fixed in d886781, and the cause is worse than a judgement call: I built run_tests.yml from a local ci.yml that was 13 commits behind origin/master, so the fifth job was never in front of me. git show origin/master:.github/workflows/ci.yml has integration-gcp-lb: at line 117.

Restored verbatim, including the vars.STREAM_GCP_API_KEY || vars.STREAM_API_KEY fallbacks, plus a timeout-minutes: 30 to match its siblings. STREAM_GCP_API_SECRET is declared required: false in the workflow_call block so the fallback still resolves.

[Should Fix] Chat and feed now run in parallel on the release path, and both mutate the same app-global setting.

spec/integration/chat_channel_integration_spec.rb:898-937 and spec/integration/feed_integration_spec.rb:899-946 do the same thing: read file_upload_config, update_app to blank it, sleep 2, assert on an upload, then restore the original in an ensure. They race: one suite's restore can land while the other is still asserting against the blanked config.

This race already existed on the PR path, where the old ci.yml also ran them as separate jobs. What is new is the release path. The old release.yml ran chat, feed and video as sequential steps inside one job, so they could not overlap there. Now the release gate calls this workflow, so a flake here fails the gate and blocks the tag.

Fix is in the specs, not this file, but it is this change that makes it reachable: scope both tests to their own app, or drop the app-global mutation and assert against a per-test channel config.

Fixed in d886781, in this file rather than the specs: integration-feed now has needs: integration-chat.

Serialising the two is a one-line change that closes the race on both paths, where scoping the tests to their own app is a spec change in two files that I would rather not fold into a release-plumbing PR. It costs the feed suite its parallelism on every PR, which is the honest trade.

Your reading of the new exposure is right and is why it is worth paying now: the old release.yml ran chat, feed and video as sequential steps in one job, so the release path could not hit this.

[Should Fix] unverified: release-pr has no needs, so on the push that merges a Release PR it runs immediately while release waits behind the full suite, up to 30 minutes.

In that window the tag does not exist yet. v12.1.0 is not reachable from master (git merge-base --is-ancestor v12.1.0 283aabc exits 1), which is exactly why last-release-sha is pinned. So during the window release-please has no release commit to stop its walk at and falls back to the pin, which means the commits the merged Release PR already consumed are still in scope. The reachable outcome is a second Release PR proposing another bump with duplicated changelog entries.

I did not read Manifest.buildPullRequests to confirm the resolution order, so this is not stated as fact. The check that settles it: merge the first release-please Release PR after this lands and watch that same run. If the Release PR job opens a new Release PR while Tests is still going, it is real.

Cheap guard either way: needs: detect plus if: needs.detect.outputs.pending != 'true', so release-pr stands down while a release is mid-flight.

Guarded in d886781, without resolving the unverified:. release-pr now has needs: detect and if: needs.detect.outputs.pending != 'true', so it stands down for the whole window rather than racing it.

Went with the guard over reading Manifest.buildPullRequests because the guard is correct whichever way the resolution order falls, and standing down while a release is mid-flight is the behaviour I want regardless. Your check still settles the question if anyone wants the answer: watch the first real Release PR merge and see whether the Release PR job would have opened another.

[Should Fix] per_page=50 with no pagination can decide there is no release to make, and pass.

The query sorts all closed PRs on the branch by updated descending and takes the first one labelled autorelease: pending. Right after a Release PR merges it is at the top, so the normal path is fine. But once a release is stuck pending (see the comment on the tag guard below), that PR slides down as other PRs close and get updated. Past 50 entries it drops off the page, pending=false, and the workflow goes green having released nothing.

A silent no-release on the workflow that pushes gems is worth closing. Query the label directly instead:

gh api "search/issues?q=repo:${GITHUB_REPOSITORY}+is:pr+is:merged+base:${GITHUB_REF_NAME}+label:%22autorelease:+pending%22"

Fixed in d886781. detect now asks for the label directly:

gh api "repos/${GITHUB_REPOSITORY}/issues" -X GET -f state=closed -f labels='autorelease: pending' -f per_page=20

then fetches that one PR for merged_at, base.ref and merge_commit_sha. Two calls, no client-side filtering, no page to fall off.

Used the issues endpoint rather than search/issues to avoid the search index lag, which would bite exactly at the moment that matters, the seconds right after a Release PR merges.

[Must Fix] One failed release run turns this workflow red on every later push to master, and burns the full suite each time.

autorelease: pending is cleared only when release-please runs in the release job below. If that job never succeeds, because tests went red (the integration suite runs against a Stream app five SDK repos share, so a flaky red is routine), or detect failed, or the run was cancelled, the merged Release PR keeps the label at its merge commit M. From then on:

1. Next push lands commit N.
2. detect still finds M, emits pending=true.
3. tests runs the whole suite on N, because line 78 gates on pending only, not on the sha.
4. This guard compares TESTED=N against PENDING=M, and exits 1.

Step 4 repeats on every push until somebody re-runs the old run for M, and nothing at the point of failure says that. The comment above says the two shas "diverge on a dispatch after the branch has moved", but this is a plain push and it diverges too.

It compounds: if a later Release PR is merged and tagged successfully, its label flips to tagged and this query selects the stale pending one again, so a healthy release does not clear the stuck state.

Fix: make the sha match part of the gate rather than a terminal error. Compare in detect, emit a second output, gate tests and release on it, and log ::warning:: when a pending release sits at a different commit. That keeps the "never tag an untested tree" property, stops spending 30 minutes per push, and leaves master green.

Fixed in d886781, along the line you sketched. The terminal guard is gone from the release job. detect now does the comparison itself and emits ready alongside pending:

• no pending release: pending=false, ready=false, nothing runs
• pending at this run's commit: both true, suite runs, tag follows
• pending at a different commit: pending=true, ready=false, and a ::warning:: naming the commit and telling you to re-run that run

tests and release gate on ready, so the stuck case costs one API call instead of the full suite plus a red X, and master stays green. The never-tag-an-untested-tree property is unchanged, it just moved from a failure into the gate.

The compounding case you describe is also closed, from the other side: release-pr now stands down while anything is pending, so a later Release PR cannot be created and merged on top of a stuck one.

[Should Fix] A PR into an N.x hotfix branch gets no tests at all.

This filter is [ master, main ], so a PR into 12.x triggers nothing. release.yml does push-trigger on '*.x', but its tests job is gated on a pending release, so the first time a hotfix is tested is the Release PR merge, when the release is already being cut. The README documents N.x as the supported hotfix path, so this is a live path with no gate.

One-line fix: branches: [ master, main, '*.x' ].

Separately, the comment above overstates the master coverage. release.yml only runs the suite when a release is pending, so an ordinary PR merge to master and a direct push to master (still possible, there is no branch protection) now run nothing. Nothing untested ships, since the Release PR merge is still gated, but the post-merge signal is gone. Worth saying that plainly in the comment rather than "master is covered by release.yml".

Both fixed in d886781. The filter is [ master, main, '*.x' ].

The comment now says what is actually true rather than claiming master is covered:

# The trade is that a merge to master, and a direct push to it,
# now run nothing: release.yml runs this same workflow only when a release is pending,
# so the gate before a tag is covered but the routine post-merge signal is gone.

[Should Fix] The manifest says 12.1.0 but lib/getstream_ruby/version.rb still says 8.0.2.

$ git show 283aabc:lib/getstream_ruby/version.rb | grep VERSION
  VERSION = '8.0.2'
$ git show v12.1.0:lib/getstream_ruby/version.rb | grep VERSION
  VERSION = '12.1.0'

The manifest value is the correct one. The tracked file is four majors stale because the old workflow committed the bump off-branch and never pushed it, which is the problem this PR exists to fix. getstream-ruby.gemspec:8 reads that constant, so master today builds a getstream-ruby-8.0.2.gem and make version reports 8.0.2.

release-please will overwrite it at the next release either way, but the first Release PR then shows 8.0.2 -> 12.1.1, which reads as corruption to whoever reviews it. Set the file to 12.1.0 in this PR so the tree matches the manifest and the tag.

Fixed in d886781, version.rb set to 12.1.0.

You are right about the reason to do it here rather than let the first Release PR carry it: 8.0.2 -> 12.1.1 in a bot PR reads as something going wrong. 12.1.0 -> 12.1.1 reads as a release. It also means master stops building a getstream-ruby-8.0.2.gem in the meantime.

[Must Fix] This states two repo settings as fact. Neither is set.

Actual settings today: allow_merge_commit: true, allow_rebase_merge: true, squash_merge_commit_title: COMMIT_OR_PR_TITLE.

The paragraph is right that both are load-bearing, which is why documenting them as done is the problem. Two ways a release goes wrong while this text says it cannot:

• A maintainer picks "Create a merge commit". The subject is Merge pull request #N from ..., not conventional, so release-please ignores the change entirely. A feat!: title in the merge body is not rescued either: splitMessages only lifts a plain feat:/fix:/... prefix out of a merge body, it has no ! branch, so a breaking change silently downgrades.
• COMMIT_OR_PR_TITLE uses the commit subject when a PR has exactly one commit. A PR titled feat: add X whose single commit says wip lands as wip. pr_title.yml passes, because it only ever looks at the PR title field, and the release does not fire.

getstream-go has allow_merge_commit: false and squash_merge_commit_title: PR_TITLE for exactly this reason.

Fix: flip the two settings before merging and keep this text, or keep the settings as they are and change the text to say what is still required. Right now a contributor reading the README has no way to know the guarantee is absent.

Fixed in d886781 by changing the text, since I have write and not admin on this repo and cannot flip the settings.

The paragraph now says squashing is a convention and not yet enforced, names the three current values, names the two required ones with getstream-go as the precedent, and spells out both failure modes: the merge commit whose body only ever yields a plain feat:, and COMMIT_OR_PR_TITLE taking a single commit's subject over the PR title. It also says pr_title.yml passes in both cases, so nobody reads a green check as protection.

The settings still need someone with admin. Flagged in the PR description too.

[Should Fix] This picks one PR before checking which branch it is on, so a release on the other branch can silently swallow this one.

.[0] takes the first label-matched PR from the whole repo. The base.ref == env.BASE check happens afterwards, in the second call. When the one picked is on a different branch, sha comes back empty and the script falls into the [ -z "$sha" ] arm, which prints "No pending release on ${BASE}" and sets pending=false. The run then goes green having released nothing.

That needs two Release PRs pending at once, which is exactly the hotfix flow the README documents: branch N.x, cherry-pick, merge the Release PR that release-please opens against it, while master still has its own pending. The repo issues list sorts by created descending, so the newer hotfix PR wins .[0] and master's release is the one dropped. It stays dropped on every later push until the other clears.

Same shape as the paging bug this replaced: a candidate is chosen before the conditions that decide whether it is valid. The unmerged-but-labelled case lands in the same arm.

Fix: filter, then pick. Ask for the numbers rather than one number, and take the first whose base matches:

nums="$(gh api "repos/${GITHUB_REPOSITORY}/issues" \
  -X GET -f state=closed -f labels='autorelease: pending' -f per_page=20 \
  --jq '.[] | select(.pull_request != null) | .number')"

sha=""
for n in $nums; do
  sha="$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${n}" \
    --jq 'select(.merged_at != null and .base.ref == env.BASE) | .merge_commit_sha // empty')"
  if [ -n "$sha" ]; then num="$n"; break; fi
done

num is used in both the warning and the success line, so it has to be set in the loop too.

Fixed in the new head. Took your loop shape: ask for the numbers, try each against base.ref, and set both num and sha on the first match so the warning and the success line still name the right PR.

You are right that it is the same shape as the paging bug, and I reintroduced it while fixing that one. The --jq producing a candidate and the validity check living in a later call is the pattern to avoid, not the specific .[0].

[Consider] needs: does not only delay feed, it skips feed whenever chat fails.

A job whose if: carries no status check function gets an implicit success() on its needs, so a red chat means feed never runs at all. That happened on this head: chat failed at chat_misc_integration_spec.rb:264 and feed reports skipped. So a chat flake now costs the feed result too, and re-running means re-running both. On the release path the effect is the same as before, since release was already skipped by a red tests, but you get less information about why.

Serialising was the right call and the trade you named in the reply is the real one. This is just the part of it that is probably not intended:

    needs: integration-chat
    if: '!cancelled()'

Feed still starts only after chat is done, so the file_upload_config windows still cannot overlap, but a chat failure no longer hides whether feed passes.

Fixed in the new head: if: '!cancelled()' alongside the needs:.

Not intended, correct. I wanted ordering and took the skip with it without noticing, and this head demonstrated it: chat red at chat_misc_integration_spec.rb:264, feed skipped. The file_upload_config windows still cannot overlap, because feed still starts only once chat is done.

For the record on that failure: the re-run of d886781 went green, so it was the propagation flake, not the diff. The test writes max_message_length: 3000 and polls 10 times at 0.5s for the read to catch up, which is a thin budget for a value that the spec itself documents as eventually consistent.