Skip to content

Isolate class-oriented workflow state across executions and replay #16

Closed

Description

@rmcdaniel

Current disposition, October 20

Delivered in PHP SDK 3.0.0, following the approved registration/container migration. Normal installed acceptance passes.

The correction merged in PR #111 at 22:17:50 UTC as 4c045e645142e0d0fa13cbd71e18dfdcb426b221. Its tree equals qualified source 7f9fd769f633e3c956282df264ddbb03a4f9dffa, preserving the original c5a4f8ce source in ancestry. Immutable tag 3.0.0 and Packagist source/dist references point to the qualified head. Class/factory registration constructs fresh workflow instances for execution, replay, queries and updates while preserving application dependency identity. Prebuilt workflow registrations and shared workflow container bindings require the documented migration. An optional factory alone, retaining legacy cloning elsewhere, would not close this defect.

Source tests, replay regression/policy checks, native Laravel/Symfony consumers and actual HTTP lifecycle checks passed. Final-head protected CI, its actual required aggregate, boundary and Sonar passed before merge; direct Sonar review found zero open findings and 0.0% new duplication. Historical source receipts retain their own exact scope.

Normal registry-installed SDK 3.0.0 passes native Laravel 13.38.0, cached Laravel configuration, Symfony 7.4.21, registration identity and real HTTP lifetime checks against immutable Server 2.5.0 and scoped-credential Server 2.9.9. Both HTTP receipts prove fresh nested/query/update state, shared dependency identity and graceful deregistration. The initial 2.9.9 setup-blocked attempt is preserved separately; after explicit bootstrap and positive readiness a fresh case passed without changing deadlines. Both PHP backends are retired. Existing SDK-2 consumers keep their constraints until deliberately migrated. Server #358's old shutdown prerequisite is complete; native/shared work remains separately owned.

Problem

The original report concerned PHP SDK 2.0.0-rc.8 resolving each attributed service once during Worker::register() and retaining bound methods from that object. Mutable properties could leak across workflow IDs, runs, task replay, queries and updates.

The later shallow-clone mitigation isolates outer scalar properties but preserves references to constructor-created nested mutable objects. Normal registry-installed 2.5.4 still fails the nested-state regression. Documentation mitigation does not complete this requirement.

Required outcome

  • Give attributed workflow handlers an explicit replay-safe lifetime so every task replay starts from clean workflow object state and independent workflow IDs or runs never share mutable workflow properties.
  • Ensure queries and updates cannot observe instance state left by another execution or replay attempt.
  • Preserve constructor dependency injection and the intentional low-level callable API without silently sharing workflow execution state.
  • Document the lifecycle distinction between workflow handlers and activity services.
  • Add behavior coverage with at least two workflow IDs and repeated replay of the same history, proving identical replay output and no cross-execution state leakage for no-container and container-resolved registration.

Completion

Close after the corrected lifecycle, migration documentation and regressions are on main, SDK 3.0.0 is published, and the exact normal installed package passes the required acceptance. Preserve historical failures and source receipts with their original scope.

Activity

  1. added this to the 2.0 beta milestone on Aug 2, 2026
  2. added
    authority:githubGitHub is the authoritative lifecycle record for this work
    kind:defectA public product behavior is incorrect
    priority:P1High-priority product or release risk
    status:readyReady for implementation
    beta:blockerMust be resolved before the first 2.0 beta
    repo:sdk-phpOwned by the PHP SDK repository
    on Aug 2, 2026
  3. rmcdaniel commented on Aug 3, 2026

    @rmcdaniel
    MemberAuthor

    Completed in 004c492ede4b on main.

    Published and verified as 2.0.0-rc.10.

    Required public qualification CI (ci.yml) passed in run 30785275433.

  4. added
    status:doneDerived from the authoritative closed issue state
    and removed
    status:readyReady for implementation
    on Aug 3, 2026
  5. rmcdaniel commented on Oct 10, 2026

    @rmcdaniel
    MemberAuthor

    Reopening because the replay-state guarantee remains incomplete for nested mutable objects.

    Independently reproduced with a fresh installation of published PHP SDK 2.2.6, source 290fa427a76ffc6ea226bcd59749374aa7a89efa. Current main 4dd3bdc49ddb18b216348e0da29f398e305586b3 still uses the same shallow-clone lifetime.

    final class Example
    {
        private object $state;
    
        public function __construct()
        {
            $this->state = (object) ['calls' => 0];
        }
    
        #[Workflow('nested-state')]
        public function run(WorkflowContext $context): int
        {
            ++$this->state->calls;
            $context->activity('step', [$this->state->calls]);
            return $this->state->calls;
        }
    }

    Register the class once through Worker::register(), then use the public WorkerTestHarness::runWorkflow() on workflow A, workflow B, and A again with the same committed activity history. Decoded completed results are 1, 2, 3, although each replay should begin with clean workflow state and return 1.

    HandlerDefinition::replaySafe() clones the outer prototype but all clones share its constructor-created state object. This is execution-local state, not an injected service. The reference documentation currently claims mutable properties cannot cross workflow IDs, runs or replay attempts.

    Remaining acceptance criteria:

    • Fresh nested execution-local state for every attributed workflow execution, replay, query and update, including container-resolved registration.
    • Preserve deliberately injected service identities. Do not blindly deep-clone database clients or other collaborators.
    • Use a fresh instance or explicit state factory with a documented lifetime, or another mechanism that actually satisfies this distinction.
    • Extend the original two-workflow and repeated-replay tests with nested mutable state and injected-service identity controls.
    • Correct the public lifetime guarantee and add the minimal reproducing replay-corpus case.
    • Verify the published successor through an ordinary consumer.

    Verification used the real SDK registration/replayer through its public harness and made no network requests. The original scalar-state fix remains useful, but it does not satisfy the full state-isolation outcome.

  6. rmcdaniel commented on Oct 10, 2026

    @rmcdaniel
    MemberAuthor

    The fresh-construction candidate is in PR #111, exact head c5a4f8ce8d7f7360858e8b5522458a5a967adb0a.

    Class registration now creates a new workflow for each task replay, query and update. WorkflowFactory supplies an explicit construction recipe for configured dependencies. Generic PSR-11 containers must return fresh workflow instances. Laravel resolves each invocation through its container. Symfony uses non-shared workflow definitions and a tagged service locator, preserving shared injected services. Activities and low-level callables keep their owned lifetimes.

    The original nested-state regression fails on main when a second execution attempts step-2 instead of the recorded step-1. Focused checks pass 25 tests and 102 assertions. All 34 exact-head checks are complete: 33 succeeded and the conditional connected-cancellation job skipped. PHP 8.1–8.4, Laravel 9–13, Symfony 6.4/7/8, embedded-to-service, package and documentation gates pass. PHP 8.3 reports 2,215 tests / 11,052 assertions with 51 framework-dependent skips. The new official Worker/replayer fixture fails on exact main 4dd3bdc4 and passes on the candidate. All 83 corpus-policy controls pass, including guards and mutations for fresh construction.

    The candidate intentionally requires a migration for prebuilt attributed workflow objects and singleton workflow bindings. The SDK cannot infer an arbitrary object's constructor arguments or decide which nested objects are local state versus injected services. A factory states that boundary explicitly. Normal class and native-framework authoring paths retain their shape.

    This is a breaking registration change, so a major-release decision is required before delivery. PR #111 remains draft. SDK main also includes shutdown #110, whose Server #358 shared/native fault and published-artifact gates remain. No version change, package release, published-image claim or issue closure is made. After the decision and qualification, install the exact successor from Composer, execute the state/identity/recovery controls and update the ordinary artifact consumers.

  7. added
    status:blockedWaiting on a named dependency or decision
    and removed
    status:doneDerived from the authoritative closed issue state
    beta:blockerMust be resolved before the first 2.0 beta
    on Oct 10, 2026
  8. rmcdaniel commented on Oct 18, 2026

    @rmcdaniel
    MemberAuthor

    The released SDK still shallow-clones attributed handler templates. Its current documentation incorrectly says this prevents mutable state crossing runs and recommends handler properties for workflow-local state. Nested objects and references can remain shared under PHP's cloning rules.

    Documentation PR #178 corrects that claim and recommends invocation-local variables and temporary objects. It preserves injected collaborator identity, explains activity/callable lifetimes and explicitly identifies the fresh-construction proposal as unreleased. No runtime, API, version or dependency changes are included. Source is 3819902fc582878718e59074a7d07a1ad2ba5a6d.

    The normal CI jobs are running. Sonar and documentation validation pass so far; this is not final delivery. I will verify the deployed docs after merge. This issue and the major-version decision for #111 remain open.

  9. rmcdaniel commented on Oct 18, 2026

    @rmcdaniel
    MemberAuthor

    Documentation mitigation delivered through #178 at 3147c3f5f5a70f06bf1bf5cf91939ad8d628f1cc, exact reviewed tree 1d86529c56584fcf3b4c2b85a879d6f81cc12e9c.

    All 36 applicable candidate checks and all 37 applicable fresh-main checks pass. The explicitly opt-in connected-cancellation job is skipped. Sonar passes with no open/confirmed PR findings and no ignored gate conditions. The developer portal deployment reports that exact main commit, and its workflow guidance now explains the shallow-clone limitation. The committed protocol and SDK reference carry the same correction.

    No SDK runtime or release changed. #16 remains open and #111 remains an unreleased major-version proposal. Invocation-local variables and temporary objects are the documented mitigation, not a claim that the handler-lifetime defect is fixed. The merged remote branch is deleted and its GitHub ref returns 404.

  10. rmcdaniel commented on Oct 20, 2026

    @rmcdaniel
    MemberAuthor

    Reopening the remaining runtime defect. The documentation mitigation in #178
    did not change the shallow-clone implementation or satisfy the nested-state
    regressions described here. Its issue closure was premature.

    The current SDK still uses clone $prototype. Draft #111 remains an unreleased
    registration change and is subject to its existing major-release decision.
    Invocation-local variables remain the available mitigation. The portal wording
    is being corrected to link that proposal without equating an issue's status
    with delivery. This does not authorize the major change or mark it qualified.

  11. rmcdaniel commented on Oct 20, 2026

    @rmcdaniel
    MemberAuthor

    Recovery assessment: registration compatibility and next decision

    I have taken ownership of the remaining recovery/disposition of #111. This review inspected published 2.5.4 source cd8834ae and held candidate c5a4f8ce; it did not change either source or rerun their cohorts.

    The current candidate needs a major release. Current discovery accepts prebuilt workflow objects. Its PSR-11 lifetime test deliberately expects one resolution and retains injected service identity. Candidate #111 instead rejects prebuilt attributed objects, repeatedly constructs workflows and refuses singleton workflow factories. Its factory/container/lifetime tests explicitly exercise those changed contracts. This affects workflows, queries, updates and the native framework bridges.

    A compatible additive factory could preserve the existing API and give opted-in applications fresh state. It would leave the original nested-state failure possible for legacy registrations, so it does not satisfy this issue's full acceptance. PHP cloning is shallow by default; universally deep-cloning or guessing constructor arguments would lose the required distinction between local state and shared collaborators. A user-defined clone hook likewise requires an application migration and cannot establish the SDK-wide guarantee. I recommend keeping the complete fresh-construction fix together rather than claiming an optional workaround fixes all registrations.

    One former blocker is obsolete. Server #358's publication receipt closes the PHP shutdown prerequisite with Server 2.5.17 / SDK 2.2.7 and actual supported-database/HTTP checks. Native rewrite parity remains separately owned; it does not keep this PHP SDK change blocked.

    Decision prepared: proceed with PHP SDK 3.0's explicit class/factory registration and transient workflow bindings, while retaining application-owned collaborator identities and activity/callable lifetimes, or keep that migration held. The major decision is required before integration/publication. No decision has been inferred from this cleanup.

    After that decision, I own the following delivery steps:

    1. Agree an exact framework-factory integration window with the active tracing change; preserve its Worker/Replayer/telemetry work and the independent signal/update drafts.
    2. Integrate current main, review the complete migration and resolve the retained Sonar gate/finding on the exact candidate. Historical normal CI is not qualification of a new tree.
    3. Run focused state/identity, workflow/query/update, invalid-factory, framework, replay-corpus, normal CI and package checks. Preserve previous failed evidence.
    4. Publish the qualified major, verify the registry artifact and repeat the changed behavior through a normal Composer consumer. Review affected consumer constraints before adoption. Sample App currently requires ^2.0; Waterline's SDK 2.4.0 is a development pin. Server and CLI have no direct SDK runtime requirement. None of those observations authorize a dependency change or deployment.

    Until the major decision, #16 and #111 remain open and explicitly blocked. Existing mitigation stays documented. No merge, tag, package, dependency pin or new full cohort was produced by this assessment.

  12. rmcdaniel commented on Oct 20, 2026

    @rmcdaniel
    MemberAuthor

    The major-release decision is approved as of October 20. I am proceeding with PHP SDK 3.0 qualification and delivery. The next action is agreeing the framework-factory source/release window with active tracing work, then integrating current main and qualifying the exact candidate described above. The prior major-decision blocker is cleared; no release or new test result is claimed by this approval.

  13. rmcdaniel commented on Oct 20, 2026

    @rmcdaniel
    MemberAuthor

    Delivered in PHP SDK 3.0.0. PR #111 merged after all required final-head checks, including the actual aggregate, at 22:17:50 UTC as 4c045e645142e0d0fa13cbd71e18dfdcb426b221. Its tree equals qualified source 7f9fd769f633e3c956282df264ddbb03a4f9dffa; immutable tag 3.0.0 points to that head.

    Normal installed acceptance, October 20, approximately 22:21–22:29 UTC:

    • Packagist lists 3.0.0 with source and dist references exactly 7f9fd769f633e3c956282df264ddbb03a4f9dffa.
    • Two clean external Composer consumers install ordinary registry SDK 3.0.0, without a path repository or source overlay. Composer locks, loaded registration identity and package references match the selected artifact. Both installs report no security vulnerability advisories.
    • Native Laravel 13.38.0 compatibility and cached configuration pass. Native Symfony 7.4.21 compatibility passes. These include the fresh-workflow factory and shared application dependency contracts.
    • Real HTTP lifetime acceptance passes against immutable Server 2.5.0 index sha256:66bd0e510c033af611cc1e2191e1ed592acf26e80f89766b0a3e1a0e463ccd19. Original workflow IDs: lifetime-9199331565a4-a and lifetime-9199331565a4-b.
    • The same installed check passes with distinct client/worker credentials against immutable Server 2.9.9 index sha256:d45d197e9556292ec096531030717e43d69ea4ea9ab197170971492f14cc0869. Original workflow IDs: lifetime-59829628ebc5-a and lifetime-59829628ebc5-b.
    • Both HTTP results positively report fresh nested state, fresh replay/query/update state, preserved shared application dependency identity and graceful worker deregistration.

    The first scoped 2.9.9 setup attempt remains recorded as runner-blocked: I omitted the documented explicit server-bootstrap step and checked liveness instead of /api/ready. No successful lifetime result is inferred from it. After bootstrap and positive readiness, the fresh workflow IDs above passed with the original helper and deadlines. The blocked original log is preserved.

    The changed lifecycle, migration guidance and regressions are on main and in the immutable artifact. Existing SDK-2 applications keep their dependency constraints until their owners deliberately migrate; no Cloud, CLI, Sample App or other downstream major adoption is included. Tracing and request-key owners will integrate actual main and run their combined checks independently. General's documentation-only markdown-it security fix is separate from this immutable PHP/Composer artifact.

    Both completed PHP test backends are retired. This closes the accepted workflow-instance isolation defect; historical source failures and their exact evidence remain preserved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    authority:githubGitHub is the authoritative lifecycle record for this workkind:defectA public product behavior is incorrectpriority:P1High-priority product or release riskrepo:sdk-phpOwned by the PHP SDK repository

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions