[Fix #1738] Replace setMetadata by fine grain manipulation. by fjtirado · Pull Request #1742 · open-workflow-specification/sdk-java

fjtirado · 2026-10-05T15:39:34Z

Fix #1738

…ain manipulation. Signed-off-by: Francisco Javier Tirado Sarti <ftirados@ibm.com>

Copilot

Copilot review overview

🟡 Changes recommended

Metadata restoration still has correctness regressions, and the public API changes break compatibility.

Review effort: Balanced
Findings: 2 · 2

Open (4)

Add a default method to preserve provider compatibility · New
Copy restored metadata into a mutable, thread-safe map · New
Preserve the public metadata() accessor for compatibility · New
Restore metadata deletions during snapshot reconciliation · New

What changed in this PR

Addresses #1738 by merging restored task metadata instead of replacing the workflow’s metadata map.

Changes:

Applies metadata entries through a shared persistence-task accessor.
Removes setMetadata() and exposes the backing map to subclasses.

File	Description
`impl/persistence/api/src/main/java/io/serverlessworkflow/impl/persistence/WorkflowPersistenceInstance.java`	Changes metadata initialization and task restoration.
`impl/persistence/api/src/main/java/io/serverlessworkflow/impl/persistence/RetriedTaskInfo.java`	Renames metadata to `additionalObjects`.
`impl/persistence/api/src/main/java/io/serverlessworkflow/impl/persistence/PersistenceTaskInfo.java`	Adds the shared metadata accessor.
`impl/core/src/main/java/io/serverlessworkflow/impl/WorkflowMutableInstance.java`	Exposes the backing map and removes the replacement setter.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

gmunozfe

Looks good to me, fantastic replacement @fjtirado !

Copilot

Copilot review overview

🟡 Changes recommended

Metadata restoration still has unresolved deletion-handling and concurrency issues.

Review effort: Balanced
Findings: 1

Open (1)

Synchronize metadata key registration and restoration · New

Resolved since last review (4)

Copy restored metadata into a mutable, thread-safe map
Add a default method to preserve provider compatibility
Restore metadata deletions during snapshot reconciliation
Preserve the public metadata() accessor for compatibility

Previously missed (1)

In code that hasn't changed since last review

Reconcile deleted metadata during snapshot restoration

impl/persistence/api/src/main/java/io/serverlessworkflow/impl/persistence/WorkflowPersistenceInstance.java:109

BytesMapInstanceTransaction.writeMetadata stores a complete durable-metadata snapshot, not a delta. If a key was removed after the workflow-start snapshot, the constructor reloads its old value and this merge leaves it present because the task snapshot omits it. Resuming therefore resurrects deleted metadata. Reconcile previously persisted keys that are absent from the task snapshot, while preserving transient metadata and keys protected during the current execution.

Copilot

Copilot review overview

🟡 Changes recommended

Restoration can still discard live metadata, undo explicit removals, and retain stale values.

Review effort: Balanced
Findings: 1 · 2

Open (3)

Concurrent metadata updates race with restoration key capture · New
No-op metadata lookup incorrectly blocks newer snapshot restoration · New
Deleted metadata keys are restored by later snapshot replay · New

Resolved since last review (1)

Synchronize metadata key registration and restoration

Copilot

Copilot review overview

🔵 Needs a closer look

Persisted-state reconciliation lacks regression coverage for nested restoration, leaving its safety insufficiently verified.

Review effort: Balanced
Findings: 1 · 1 · 1

Open (3)

Concurrent metadata updates race with restoration key capture
Deleted metadata keys are restored by later snapshot replay
Add regression coverage for metadata preservation during child restoration · New

Resolved since last review (1)

No-op metadata lookup incorrectly blocks newer snapshot restoration

Copilot

Copilot review overview

🔵 Needs a closer look

Concurrent restoration and metadata ownership changes need human validation before approval.

Review effort: Balanced
Findings: 1 · 1

Open (2)

Concurrent metadata updates race with restoration key capture
Add regression coverage for metadata preservation during child restoration

Resolved since last review (1)

Deleted metadata keys are restored by later snapshot replay

Copilot

Copilot review overview

🟡 Changes recommended

The restored metadata map is no longer thread-safe, allowing concurrent updates to fail persistence snapshotting.

Review effort: Balanced
Findings: 1 · 1

Open (2)

Replacing ConcurrentHashMap with HashMap breaks concurrent metadata reads · New
Add regression coverage for metadata preservation during child restoration

Resolved since last review (1)

Concurrent metadata updates race with restoration key capture

…riding keys set by user Signed-off-by: Francisco Javier Tirado Sarti <ftirados@ibm.com>

Copilot

Copilot review overview

🔵 Needs a closer look

Metadata preservation and deletion during recovery lack focused regression coverage and need human validation.

Review effort: Balanced
Findings: 2

Open (2)

Add regression tests for metadata restoration and add-if-absent protection · New
Add regression coverage for metadata preservation during child restoration

Resolved since last review (1)

Replacing ConcurrentHashMap with HashMap breaks concurrent metadata reads

[Fix open-workflow-specification#1738] Replace setMetadata by fine gr…

de5f360

…ain manipulation. Signed-off-by: Francisco Javier Tirado Sarti <ftirados@ibm.com>

Copilot AI balanced review requested due to automatic review settings October 5, 2026 15:39

Copilot started reviewing on behalf of fjtirado October 5, 2026 15:40 View session

fjtirado requested a review from gmunozfe October 5, 2026 15:42

Copilot AI reviewed Oct 5, 2026

View reviewed changes

gmunozfe approved these changes Oct 5, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings October 5, 2026 19:56

Copilot started reviewing on behalf of fjtirado October 5, 2026 20:07 View session

fjtirado force-pushed the Fix_#1738 branch from c977a8a to 09647b7 Compare October 5, 2026 20:08

Copilot AI reviewed Oct 5, 2026

View reviewed changes

Comment thread ...ce/api/src/main/java/io/serverlessworkflow/impl/persistence/WorkflowPersistenceInstance.java Outdated

fjtirado force-pushed the Fix_#1738 branch from 09647b7 to 00c94c8 Compare October 5, 2026 20:13

Copilot AI balanced review requested due to automatic review settings October 5, 2026 20:13

Copilot started reviewing on behalf of fjtirado October 5, 2026 20:14 View session

fjtirado commented Oct 5, 2026

View reviewed changes

Comment thread ...ce/api/src/main/java/io/serverlessworkflow/impl/persistence/WorkflowPersistenceInstance.java

fjtirado force-pushed the Fix_#1738 branch from 00c94c8 to 199dfcb Compare October 5, 2026 20:19

Copilot AI reviewed Oct 5, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings October 5, 2026 20:20

Copilot started reviewing on behalf of fjtirado October 5, 2026 20:21 View session

Copilot AI reviewed Oct 5, 2026

View reviewed changes

Comment thread ...ce/api/src/main/java/io/serverlessworkflow/impl/persistence/WorkflowPersistenceInstance.java Outdated

Copilot AI balanced review requested due to automatic review settings October 5, 2026 20:34

fjtirado force-pushed the Fix_#1738 branch from 199dfcb to b9e7c2c Compare October 5, 2026 20:34

Copilot AI reviewed Oct 5, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings October 5, 2026 20:54

fjtirado force-pushed the Fix_#1738 branch from b9e7c2c to c9900a2 Compare October 5, 2026 20:54

Copilot started reviewing on behalf of fjtirado October 5, 2026 20:55 View session

Copilot AI reviewed Oct 5, 2026

View reviewed changes

Comment thread ...ce/api/src/main/java/io/serverlessworkflow/impl/persistence/WorkflowPersistenceInstance.java Outdated

[Fix open-workflow-specification#1738] Adding safeguard to avoid over…

e0c734a

…riding keys set by user Signed-off-by: Francisco Javier Tirado Sarti <ftirados@ibm.com>

Copilot AI balanced review requested due to automatic review settings October 5, 2026 21:03

fjtirado force-pushed the Fix_#1738 branch from c9900a2 to e0c734a Compare October 5, 2026 21:03

Copilot AI reviewed Oct 5, 2026

View reviewed changes

Comment thread ...ce/api/src/main/java/io/serverlessworkflow/impl/persistence/WorkflowPersistenceInstance.java

fjtirado merged commit 45b5391 into open-workflow-specification:main Oct 5, 2026
4 checks passed

Copilot started reviewing on behalf of fjtirado October 5, 2026 22:10 View session

Copilot started reviewing on behalf of fjtirado October 5, 2026 22:21 View session

wmedvede reviewed Oct 6, 2026

View reviewed changes

Conversation

fjtirado commented Oct 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gmunozfe left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants