Skip to content

ci: ping Slack when a resume needs approval - #317

Merged
ioannisj merged 5 commits into
mainfrom
ci/release-resume-slack-approval
Nov 12, 2026
Merged

ioannisj merged 5 commits into
mainfrom
ci/release-resume-slack-approval

Conversation

@ioannisj

@ioannisj ioannisj commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

💡 Motivation and Context

Follow-up to #316. The first real resume (run 43037612563, finishing Laravel 0.4.1) sat waiting on the Release approval with no Slack message at all. notify-approval-needed is gated on pending change intents, so a resume_run_id dispatch skips it, and the only Slack replies the resume sends come after approval.

Reuses the two Slack paths the workflow already has instead of adding a third:

  • When the resumed run's release-slack-thread-* artifact is still there, resume-check posts one slack-thread-reply in that thread once its checks pass: the @support-hero-client-libraries mention and a link to the resume run. Same action and pin the approved, released, and rejected replies use. It adds an hourglass reaction on the approval message, and the approved reply removes it.
  • When the artifact is gone, the existing notify-approval-needed job runs for the resume too, so it gets the normal approval message and button, and preserve-slack-thread saves the new thread. resume-publication and notify-released reply under whichever thread exists.

Everything in the message is already validated upstream: resume_run_id has to match ^[0-9]+$ and the versions have to match ^[0-9]+\.[0-9]+\.[0-9]+$ in scripts/check-release-refs.sh before the step runs. The Slack step is continue-on-error, so a Slack outage never blocks a recovery.

A failed resume was already reported by release-failure-notification.yml, which reads the same thread artifact, so nothing changes there. RELEASING.md describes the ping in the "Partial publication" section.

💚 How did you test it?

  • actionlint with shellcheck on release.yml, clean.
  • Evaluated the job conditions for four runs: a normal release, a resume with the thread artifact, a resume without it, and a push with no change intents. The resumes never reach release, the normal runs never reach the resume jobs, and notify-approval-needed runs only for a normal release or a thread-less resume.
  • Rendered the thread message for the 0.4.1 resume: mention, link to run 43037612563, "completes PHP core 4.23.1 and Laravel 0.4.1 from run 43015053422".
  • Not exercised end to end: the next real resume dispatch will be the first live post.

📝 Checklist

  • All commits are signed (see Commit signing)
  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

If releasing new changes

  • Ran pnpm change to generate a change intent file

🤖 Agent context

DRI: @ioannisj
Autonomy: Human-driven (agent-assisted)

@ioannisj
ioannisj requested a review from a team as a code owner November 11, 2026 15:04
@ioannisj ioannisj self-assigned this Nov 11, 2026
@veria-ai

veria-ai Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

PR overview

No security issues were found in this pull request.

Reviews

@marandaneto

Copy link
Copy Markdown
Member

make sure to reuse our own actions such as posthog/.github/.github/workflows/notify-approval-needed.yml instead of rewriting the whole thing

@github-actions

github-actions Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

posthog-php-lib_curl Compliance Report

Date: 2026-11-11T18:12:50.112429+00:00
Duration: 116482ms

⚠️ Some Tests Failed

46/47 tests passed, 1 failed


Capture Tests

⚠️ 29/30 tests passed, 1 failed

View Details
Test Status Duration
Format Validation.Event Has Required Fields ✅ 26ms
Format Validation.Event Has Uuid ✅ 521ms
Format Validation.Event Has Lib Properties ✅ 522ms
Format Validation.Distinct Id Is String ✅ 523ms
Format Validation.Token Is Present ✅ 523ms
Format Validation.Custom Properties Preserved ✅ 522ms
Format Validation.Event Has Timestamp ✅ 523ms
Format Validation.Non Utc Event Timestamp Is Converted To Utc ✅ 523ms
Retry Behavior.Retries On 503 ✅ 5832ms
Retry Behavior.Does Not Retry On 400 ✅ 2526ms
Retry Behavior.Does Not Retry On 401 ✅ 2524ms
Retry Behavior.Respects Retry After Header ❌ 6334ms
Retry Behavior.Implements Backoff ✅ 16245ms
Retry Behavior.Retries On 500 ✅ 5631ms
Retry Behavior.Retries On 502 ✅ 5632ms
Retry Behavior.Retries On 504 ✅ 5630ms
Retry Behavior.Max Retries Respected ✅ 17045ms
Deduplication.Generates Unique Uuids ✅ 228ms
Deduplication.Preserves Uuid On Retry ✅ 5631ms
Deduplication.Preserves Uuid And Timestamp On Retry ✅ 10836ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry ✅ 5634ms
Deduplication.No Duplicate Events In Batch ✅ 528ms
Deduplication.Different Events Have Different Uuids ✅ 524ms
Compression.Sends Gzip When Enabled ✅ 522ms
Batch Format.Uses Proper Batch Structure ✅ 523ms
Batch Format.Flush With No Events Sends Nothing ✅ 518ms
Batch Format.Multiple Events Batched Together ✅ 511ms
Error Handling.Does Not Retry On 403 ✅ 2524ms
Error Handling.Does Not Retry On 413 ✅ 2524ms
Error Handling.Retries On 408 ✅ 5630ms

Failures

retry_behavior.respects_retry_after_header

Retry delay too short: 804ms < 2500ms

Feature_Flags Tests

✅ 17/17 tests passed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id ✅ 523ms
Request Payload.Flags Request Uses V2 Query Param ✅ 520ms
Request Payload.Flags Request Hits Flags Path Not Decide ✅ 520ms
Request Payload.Flags Request Omits Authorization Header ✅ 520ms
Request Payload.Token In Flags Body Matches Init ✅ 521ms
Request Payload.Groups Round Trip ✅ 521ms
Request Payload.Groups Default To Empty Object ✅ 520ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False ✅ 520ms
Request Payload.Disable Geoip Omitted Defaults To False ✅ 520ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key ✅ 521ms
Request Lifecycle.No Flags Request On Init Alone ✅ 516ms
Request Lifecycle.No Flags Request On Normal Capture ✅ 507ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests ✅ 523ms
Request Lifecycle.Mock Response Value Is Returned To Caller ✅ 522ms
Retry Behavior.Retries Flags On 502 ✅ 623ms
Retry Behavior.Retries Flags On 504 ✅ 630ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event ✅ 521ms

@github-actions

github-actions Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

posthog-php-fork_curl Compliance Report

Date: 2026-11-11T18:12:49.943416+00:00
Duration: 116197ms

⚠️ Some Tests Failed

46/47 tests passed, 1 failed


Capture Tests

⚠️ 29/30 tests passed, 1 failed

View Details
Test Status Duration
Format Validation.Event Has Required Fields ✅ 58ms
Format Validation.Event Has Uuid ✅ 527ms
Format Validation.Event Has Lib Properties ✅ 535ms
Format Validation.Distinct Id Is String ✅ 533ms
Format Validation.Token Is Present ✅ 534ms
Format Validation.Custom Properties Preserved ✅ 536ms
Format Validation.Event Has Timestamp ✅ 534ms
Format Validation.Non Utc Event Timestamp Is Converted To Utc ✅ 538ms
Retry Behavior.Retries On 503 ✅ 5862ms
Retry Behavior.Does Not Retry On 400 ✅ 2537ms
Retry Behavior.Does Not Retry On 401 ✅ 2536ms
Retry Behavior.Respects Retry After Header ❌ 6351ms
Retry Behavior.Implements Backoff ✅ 16274ms
Retry Behavior.Retries On 500 ✅ 5162ms
Retry Behavior.Retries On 502 ✅ 5666ms
Retry Behavior.Retries On 504 ✅ 5647ms
Retry Behavior.Max Retries Respected ✅ 16286ms
Deduplication.Generates Unique Uuids ✅ 542ms
Deduplication.Preserves Uuid On Retry ✅ 5651ms
Deduplication.Preserves Uuid And Timestamp On Retry ✅ 10863ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry ✅ 5654ms
Deduplication.No Duplicate Events In Batch ✅ 543ms
Deduplication.Different Events Have Different Uuids ✅ 536ms
Compression.Sends Gzip When Enabled ✅ 538ms
Batch Format.Uses Proper Batch Structure ✅ 534ms
Batch Format.Flush With No Events Sends Nothing ✅ 522ms
Batch Format.Multiple Events Batched Together ✅ 521ms
Error Handling.Does Not Retry On 403 ✅ 2536ms
Error Handling.Does Not Retry On 413 ✅ 2535ms
Error Handling.Retries On 408 ✅ 5650ms

Failures

retry_behavior.respects_retry_after_header

Retry delay too short: 811ms < 2500ms

Feature_Flags Tests

✅ 17/17 tests passed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id ✅ 532ms
Request Payload.Flags Request Uses V2 Query Param ✅ 523ms
Request Payload.Flags Request Hits Flags Path Not Decide ✅ 523ms
Request Payload.Flags Request Omits Authorization Header ✅ 525ms
Request Payload.Token In Flags Body Matches Init ✅ 524ms
Request Payload.Groups Round Trip ✅ 524ms
Request Payload.Groups Default To Empty Object ✅ 525ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False ✅ 524ms
Request Payload.Disable Geoip Omitted Defaults To False ✅ 525ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key ✅ 524ms
Request Lifecycle.No Flags Request On Init Alone ✅ 519ms
Request Lifecycle.No Flags Request On Normal Capture ✅ 518ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests ✅ 529ms
Request Lifecycle.Mock Response Value Is Returned To Caller ✅ 526ms
Retry Behavior.Retries Flags On 502 ✅ 628ms
Retry Behavior.Retries Flags On 504 ✅ 627ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event ✅ 535ms

@github-actions

github-actions Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

posthog-php-socket Compliance Report

Date: 2026-11-11T18:13:04.195197+00:00
Duration: 115983ms

⚠️ Some Tests Failed

46/47 tests passed, 1 failed


Capture Tests

⚠️ 29/30 tests passed, 1 failed

View Details
Test Status Duration
Format Validation.Event Has Required Fields ✅ 24ms
Format Validation.Event Has Uuid ✅ 521ms
Format Validation.Event Has Lib Properties ✅ 522ms
Format Validation.Distinct Id Is String ✅ 522ms
Format Validation.Token Is Present ✅ 522ms
Format Validation.Custom Properties Preserved ✅ 523ms
Format Validation.Event Has Timestamp ✅ 528ms
Format Validation.Non Utc Event Timestamp Is Converted To Utc ✅ 522ms
Retry Behavior.Retries On 503 ✅ 5831ms
Retry Behavior.Does Not Retry On 400 ✅ 2526ms
Retry Behavior.Does Not Retry On 401 ✅ 2524ms
Retry Behavior.Respects Retry After Header ❌ 6330ms
Retry Behavior.Implements Backoff ✅ 16246ms
Retry Behavior.Retries On 500 ✅ 5632ms
Retry Behavior.Retries On 502 ✅ 5632ms
Retry Behavior.Retries On 504 ✅ 5633ms
Retry Behavior.Max Retries Respected ✅ 16246ms
Deduplication.Generates Unique Uuids ✅ 528ms
Deduplication.Preserves Uuid On Retry ✅ 5630ms
Deduplication.Preserves Uuid And Timestamp On Retry ✅ 10837ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry ✅ 5632ms
Deduplication.No Duplicate Events In Batch ✅ 527ms
Deduplication.Different Events Have Different Uuids ✅ 524ms
Compression.Sends Gzip When Enabled ✅ 523ms
Batch Format.Uses Proper Batch Structure ✅ 523ms
Batch Format.Flush With No Events Sends Nothing ✅ 519ms
Batch Format.Multiple Events Batched Together ✅ 510ms
Error Handling.Does Not Retry On 403 ✅ 2526ms
Error Handling.Does Not Retry On 413 ✅ 2523ms
Error Handling.Retries On 408 ✅ 5630ms

Failures

retry_behavior.respects_retry_after_header

Retry delay too short: 802ms < 2500ms

Feature_Flags Tests

✅ 17/17 tests passed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id ✅ 524ms
Request Payload.Flags Request Uses V2 Query Param ✅ 521ms
Request Payload.Flags Request Hits Flags Path Not Decide ✅ 521ms
Request Payload.Flags Request Omits Authorization Header ✅ 520ms
Request Payload.Token In Flags Body Matches Init ✅ 521ms
Request Payload.Groups Round Trip ✅ 521ms
Request Payload.Groups Default To Empty Object ✅ 520ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False ✅ 524ms
Request Payload.Disable Geoip Omitted Defaults To False ✅ 517ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key ✅ 520ms
Request Lifecycle.No Flags Request On Init Alone ✅ 525ms
Request Lifecycle.No Flags Request On Normal Capture ✅ 502ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests ✅ 524ms
Request Lifecycle.Mock Response Value Is Returned To Caller ✅ 521ms
Retry Behavior.Retries Flags On 502 ✅ 624ms
Retry Behavior.Retries Flags On 504 ✅ 623ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event ✅ 522ms

@marandaneto

Copy link
Copy Markdown
Member

We can simplify this by reusing the notification flow already in .github/workflows/release.yml:

  1. Run the existing notify-approval-needed job (which calls posthog/.github/.github/workflows/notify-approval-needed.yml) after either normal candidate verification or a successful resume-check. The condition needs always() && !cancelled() plus explicit success checks, since the jobs for the other path are skipped.
  2. Reuse preserve-slack-thread to save the returned slack_ts.
  3. Wire the resume approval/success replies to that notification timestamp.
  4. Remove the duplicated Slack payload construction and new-thread handling from resume-check.

The trade-off is a fresh approval thread for each resume: the shared workflow currently has no inputs for replying in an existing thread or rendering the custom publication-status fields. If retaining those features is required, please extend the shared workflow first and reuse it here rather than duplicate the message implementation.

Could you revisit the PR with that approach?

@ioannisj

Copy link
Copy Markdown
Contributor Author

🤖 An AI agent wrote this on behalf of @ioannisj.

Reworked in 641933f and 6bbe249, the custom payload is gone.

  • When the resumed run's thread artifact is still there, resume-check posts one slack-thread-reply in that thread (the same action the approved and released replies use): the support-hero mention plus a link to the resume run. No blocks, no button.
  • When the artifact is gone, the existing notify-approval-needed job runs for the resume too, so it gets the normal message and preserve-slack-thread saves the new thread. resume-publication and notify-released reply under whichever one exists.

I kept the thread reply because a resume happens hours after the original approval and the question for the approver is the same one, so it reads better in that thread than as a fresh one each time. It also means we don't have to add a thread_ts input to the shared notify-approval-needed workflow just for this.

@marandaneto marandaneto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The code looks good; no qualifying findings. Verdict: correct.

@ioannisj
ioannisj merged commit ff58bda into main Nov 12, 2026
35 checks passed
@ioannisj
ioannisj deleted the ci/release-resume-slack-approval branch November 12, 2026 09:33
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.

2 participants