chore: auto-sync develop → main (direct push failure) - #116
Closed
brendanobra wants to merge 15 commits into
Closed
Conversation
Chore: one time sync of main -> develop to prepare for ff automation
RDKEMW-19316 : Add coding guidelines for firebolt-cpp-transport
…cation * Replace SubscriptionData value map with shared_ptr; drop stale notifications via weak_ptr * Address copilot comments * Address copilot comments * Catch any_cast errors in onPropertyChangedCallback * Address copilot comments
RDKEMW-22177: update automation to use new PAT
Contributor
There was a problem hiding this comment.
Pull request overview
This PR syncs develop into main after an automated direct-push sync failed, bringing in a transport-layer safety fix for event notification dispatch plus associated regression tests and CI workflow token updates.
Changes:
- Harden subscription notification dispatch against use-after-free by guarding
HelperImplsubscription callbacks with aweak_ptr. - Prevent the notification worker thread from terminating when callbacks throw by catching exceptions during dispatch, and add regression tests for both behaviors.
- Update GitHub Actions workflows to use
secrets.AUTOMATION_TOKEN, and replace the prior Copilot instruction file with a consolidated coding-guidelines instruction document.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/helpers_impl.h |
Stores subscriptions as shared_ptr and wraps callbacks with a weak_ptr guard to drop in-flight notifications after unsubscribe. |
include/firebolt/helpers.h |
Improves robustness of onPropertyChangedCallback by moving any_cast into the try block and adding a bad_any_cast handler. |
src/gateway.cpp |
Adds exception handling around notification callback dispatch to keep the worker thread alive. |
test/unit/helperTest.cpp |
Adds regression tests validating the weak_ptr guard behavior across unsubscribe paths. |
test/unit/gatewayTest.cpp |
Adds regression test ensuring notification worker continues after a callback throws, plus required header include. |
.github/workflows/sync-develop-to-main.yml |
Switches workflow auth to secrets.AUTOMATION_TOKEN for checkout/PR creation. |
.github/workflows/release.yml |
Switches semantic-release auth to secrets.AUTOMATION_TOKEN. |
.github/instructions/coding-guidelines.instructions.md |
Adds consolidated repository coding guidelines for tooling/agents. |
.github/copilot-instructions.md |
Removes the older Copilot instructions document (superseded by the new guidelines). |
Comment on lines
+82
to
85
| void* notificationPtr = static_cast<void*>(it->second.get()); | ||
| auto errorStatus{gateway_.unsubscribe(it->second->eventName, notificationPtr)}; | ||
| subscriptions_.erase(it); | ||
| return Result<void>{errorStatus}; |
Comment on lines
43
to
47
| try | ||
| { | ||
| auto notifier = std::any_cast<std::function<void(Args...)>>(subscriptionData->notification); | ||
| JsonType jsonType; | ||
| jsonType.fromJson(jsonResponse); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automatic sync of
develop→maincould not complete via direct workflow push.Action required: resolve the conflicts in this PR, then merge it.
Detected reason: direct push failure
Triggered by: daa7cb7 on develop