Repository navigation
ci: ping Slack when a resume needs approval - #317
Conversation
|
make sure to reuse our own actions such as posthog/.github/.github/workflows/notify-approval-needed.yml instead of rewriting the whole thing |
posthog-php-lib_curl Compliance ReportDate: 2026-11-11T18:12:50.112429+00:00
|
| 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 |
posthog-php-fork_curl Compliance ReportDate: 2026-11-11T18:12:49.943416+00:00
|
| 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 |
posthog-php-socket Compliance ReportDate: 2026-11-11T18:13:04.195197+00:00
|
| 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 |
|
We can simplify this by reusing the notification flow already in
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? |
Reworked in 641933f and 6bbe249, the custom payload is gone.
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 |
marandaneto
left a comment
There was a problem hiding this comment.
The code looks good; no qualifying findings. Verdict: correct.
💡 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-neededis gated on pending change intents, so aresume_run_iddispatch 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:
release-slack-thread-*artifact is still there,resume-checkposts oneslack-thread-replyin that thread once its checks pass: the@support-hero-client-librariesmention 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.notify-approval-neededjob runs for the resume too, so it gets the normal approval message and button, andpreserve-slack-threadsaves the new thread.resume-publicationandnotify-releasedreply under whichever thread exists.Everything in the message is already validated upstream:
resume_run_idhas to match^[0-9]+$and the versions have to match^[0-9]+\.[0-9]+\.[0-9]+$inscripts/check-release-refs.shbefore the step runs. The Slack step iscontinue-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?
actionlintwith shellcheck onrelease.yml, clean.release, the normal runs never reach the resume jobs, andnotify-approval-neededruns only for a normal release or a thread-less resume.📝 Checklist
If releasing new changes
pnpm changeto generate a change intent file🤖 Agent context
DRI: @ioannisj
Autonomy: Human-driven (agent-assisted)