Clear queued writes when clearing an unpublished scope

[PratikDhanave]

StateManager.ClearStateByID early-returned whenever the scope had no committed entry in sm.scopes:

scope, exists := sm.scopes.Load(scopeID)
if !exists {
    return nil
}
keysToDelete := scope.ReadKeys()
// ... loop that converts queued writes into deletes ...

But a scope only materializes in sm.scopes once published. Writes made during the current superstep live only in queuedUpdates (via WriteStateByID) and do not create the scope. So when an executor writes a key and then clears the scope within the same superstep — before any read (ReadStateByID → getOrCreateScope) or publish has materialized it — the early return fires and the queued-write-to-delete loop never runs.

Result: the pending write silently survives the clear. ReadStateByID (which consults queuedUpdates first) still returns it, and PublishUpdates commits it. Reachable from the public API via inproc QueueStateUpdate→WriteState and QueueClearScope→ClearState — an executor doing "stage some state, then decide to clear the scope."

The function’s own body already iterates the queued updates to turn them into deletes, so the early return defeats its documented intent only in the not-yet-materialized-scope case. Existing state_test.go paths always publish before clearing, so the gap was untested.

Fix

Drop the early return; run the clearing loop unconditionally, treating a missing scope as an empty committed key set:

keysToDelete := map[string]struct{}{}
if scope, exists := sm.scopes.Load(scopeID); exists {
    keysToDelete = scope.ReadKeys()
}

Test

TestStateManager_ClearByIDDropsUnpublishedWriteOnFreshScope writes a key to a never-published scope, clears it, and asserts the key is absent both before and after PublishUpdates. It fails on the old code (the write survives the clear) and passes with the fix.

[Copilot]

Pull request overview

This pull request fixes a state-clearing edge case where clearing an unpublished scope could previously fail to convert same-superstep queued writes into deletes, allowing a “write-then-clear” sequence to survive and be committed on publish. It also adds basic synchronization to the in-memory checkpoint manager to avoid concurrent map/cache races, plus tests to cover both behaviors.

Changes:

• Update StateManager.ClearStateByID to avoid early-returning when the scope hasn’t been materialized in sm.scopes, ensuring queued writes are still cleared.
• Add a regression test that exercises “write then clear before publish” on a fresh/unpublished scope.
• Add a mutex to the in-memory checkpoint manager and a concurrent-sessions test to validate the absence of races (when run with -race).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
workflow/internal/execution/state.go	Removes the premature early return so clears also apply to queued (unpublished) updates.
workflow/internal/execution/state_test.go	Adds a regression test for clearing a never-published scope with queued writes.
workflow/checkpoint/manager.go	Adds locking to make the in-memory manager safe for concurrent session usage.
workflow/checkpoint/manager_test.go	Adds a concurrency test for the in-memory manager (currently contains a loop that doesn’t compile).

---

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

[PratikDhanave]

That file was included by accident (this branch had been cut from #522); I rebased so this PR now contains only the state.go fix. (Also, range-over-int is valid since Go 1.22, which this module targets.)