Skip to content

Signal race condition#273

Merged
rmcdaniel merged 9 commits intomasterfrom
signal-race-condition
Oct 28, 2025
Merged

Signal race condition#273
rmcdaniel merged 9 commits intomasterfrom
signal-race-condition

Conversation

@rmcdaniel
Copy link
Copy Markdown
Member

No description provided.

@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (971239e) to head (2456aa1).
⚠️ Report is 10 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##              master      #273   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
  Complexity       310       310           
===========================================
  Files             46        46           
  Lines           1290      1297    +7     
===========================================
+ Hits            1290      1297    +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@rmcdaniel rmcdaniel requested a review from Copilot October 28, 2025 16:44
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses a signal race condition in workflow execution by adjusting how signals are processed relative to workflow logs. The key change ensures signals are processed up to the next log entry rather than the current one, preventing signals from being missed when they arrive between workflow steps.

Key Changes:

  • Modified signal processing to use the next log entry's timestamp as the cutoff point instead of the current log entry
  • Optimized database queries by fetching current and next log entries in a single query
  • Added comprehensive test coverage for both early and late signal delivery scenarios

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/Workflow.php Updated signal processing logic to use next log entry timestamp and optimized log queries to fetch both current and next entries together
tests/Feature/WorkflowTest.php Added four test cases covering early/late signal delivery scenarios for two workflow types
tests/Fixtures/TestSignalExceptionWorkflow.php New test fixture implementing a workflow that handles signals after activity exceptions
tests/Fixtures/TestSignalExceptionWorkflowLeader.php New test fixture implementing a simpler workflow variant for signal exception testing
tests/Fixtures/TestSingleTryExceptionActivity.php New test activity that throws exceptions on first try to simulate failure scenarios

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/Fixtures/TestSingleTryExceptionActivity.php
Comment thread tests/Fixtures/TestSingleTryExceptionActivity.php
Comment thread tests/Feature/WorkflowTest.php
Comment thread tests/Feature/WorkflowTest.php
Comment thread tests/Feature/WorkflowTest.php
Comment thread tests/Feature/WorkflowTest.php
@rmcdaniel rmcdaniel merged commit aa9424f into master Oct 28, 2025
3 of 4 checks passed
@rmcdaniel rmcdaniel deleted the signal-race-condition branch October 28, 2025 18:45
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