Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4a8a12c68
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| $completion->recordHeartbeat( | ||
| $execution->getID(), | ||
| $execution->getRunID(), | ||
| $scheduled->getActivityId(), |
There was a problem hiding this comment.
When this async-completion feature reaches this block, recordHeartbeat() and the later complete() are the task-token forms of the PHP activity completion client, but the arguments supplied here are workflow, run, and activity IDs. The workflow ID is therefore sent as an invalid task token (and complete() is invoked with the same wrong shape), so the feature fails before checking any serialization context; call the client's by-ID variants or publish and use the actual task token.
Useful? React with 👍 / 👎.
| ): void { | ||
| $firstExecution = $stub->getExecution(); | ||
| Assert::same($stub->getResult()->value, FINAL_RESULT); | ||
| $lastExecution = $stub->getExecution(); |
There was a problem hiding this comment.
For this workflow's one continue-as-new, getResult() follows the run chain to obtain the result but does not replace the execution associated with the stub, so this returns the same first-run execution captured on line 66. The history request on line 79 consequently reads the first run again and cannot find WorkflowExecutionCompleted; construct an execution with the workflow ID and no run ID, or use the new run ID from the continued-as-new event.
Useful? React with 👍 / 👎.
| return yield Workflow::executeActivity( | ||
| ACTIVITY_NAME, | ||
| [$input], | ||
| LocalActivityOptions::new()->withStartToCloseTimeout(10), |
There was a problem hiding this comment.
When the PHP local-activity feature runs, Workflow::executeActivity() is the remote-activity entry point and does not turn into a local activity when passed LocalActivityOptions (the SDK rejects that option type). The workflow therefore fails before producing a LocalActivity marker, so this conformance case can never inspect an isLocal=true context; use Workflow::executeLocalActivity() here.
Useful? React with 👍 / 👎.
…on-context-php # Conflicts: # harness/php/composer.json
1 participant
What was changed
Why?
Checklist
Closes
How was this tested: