Skip to content

Conversation

@rustatian
Copy link
Collaborator

@rustatian rustatian commented Nov 6, 2025

Reason for This PR

Description of Changes

  • Pass WW PID to every command via the message field.
  • Return PHP WF worker pool restart on reset.

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the MIT license.

PR Checklist

[Author TODO: Meet these criteria.]
[Reviewer TODO: Verify that these criteria are met. Request changes if not]

  • All commits in this PR are signed (git commit -s).
  • The reason for this PR is clearly provided (issue no. or explanation).
  • The description of changes is clear and encompassing.
  • Any required documentation changes (code and docs) are included in this PR.
  • Any user-facing changes are mentioned in CHANGELOG.md.
  • All added/changed functionality is tested.

Signed-off-by: Valery Piashchynski <[email protected]>
@rustatian rustatian requested a review from Copilot November 6, 2025 10:13
@rustatian rustatian self-assigned this Nov 6, 2025
@rustatian rustatian added B-missing feature Bug: missing feature after implementation C-feature-request Category: feature requested, but need to be discussed labels Nov 6, 2025
Copy link

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 adds workflow worker PID tracking to prevent sending responses from dead workers, and includes minor comment improvements. The changes help ensure that messages are tagged with the correct worker PID to handle worker lifecycle scenarios more gracefully.

Key Changes

  • Added a WorkflowWorkerPID field to the Message structure with JSON serialization support
  • Updated PushCommand and AllocateMessage methods to accept and set the workflow worker PID
  • Fixed minor grammatical issues in code comments

Reviewed Changes

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

Show a summary per file
File Description
internal/protocol.go Added WorkflowWorkerPID field to the Message struct for tracking worker identity
queue/queue.go Updated queue methods to accept and populate the workflow worker PID parameter
queue/queue_test.go Updated test to pass the new required PID parameter to PushCommand
aggregatedpool/workflow.go Added logic to retrieve and pass workflow worker PID when pushing commands; fixed comment grammar
aggregatedpool/handler.go Added workflow worker PID retrieval logic across multiple handler methods
aggregatedpool/workers.go Fixed extra whitespace in comment
go.work.sum Updated dependency checksums

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

Signed-off-by: Valery Piashchynski <[email protected]>
Signed-off-by: Valery Piashchynski <[email protected]>
Signed-off-by: Valery Piashchynski <[email protected]>
Signed-off-by: Valery Piashchynski <[email protected]>
Signed-off-by: Valery Piashchynski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-missing feature Bug: missing feature after implementation C-feature-request Category: feature requested, but need to be discussed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[🐛 BUG]: rr reset not restart main temporal pid

2 participants