fix(repositories): stop the shared-pointer live edit in MemoryGraphRepository (A-06)

[gustavobertoi]

Summary

BACKLOG_V2.md A-06 · Stop the shared-pointer live edit (= F-01 finding 3).

MemoryGraphRepository.FindByID (internal/repositories/graph_memory.go) returned the
exact *workflow.Graph pointer stored by Save — the same object
internal/actors/workflow_handler.go hands to a triggered Workflow. A later
GraphService.Upsert → updateVersioned → Graph.UpdateSchema
(internal/services/graph_service.go:244-284, internal/workflow/graph.go:122-133) for the
same schema id then rewrote that in-flight run's topology in place: a forking node could
lose a branch mid-run, and Workflow.Next could nil-deref a node deleted out from under it
(internal/workflow/workflow.go:312-314).

The Postgres driver was already immune — postgres/graph.go:FindByID rebuilds a fresh
Graph from the object store on every call.

Fix

FindByID now returns a fresh *workflow.Graph built from a cloned Schema():

schema := graph.Schema()
return workflow.NewGraph(&schema)

This mirrors the existing FindByIDAndVersion idiom in the same file verbatim
(schema := sv.Schema.Clone(); workflow.NewGraph(&schema)), and needs no change to
updateVersioned, SetActiveVersion, or Rollback — the latter two already build fresh
Graph values from a cloned schema and are out of scope here (that territory belongs to
A-05). A grep of every GraphRepository.FindByID / GraphService.FindByID call site
(cron_scheduler.go, event_trigger.go, workflow_sup.go, webhook_router.go,
workflow_handler.go:168, workflow_schema.go, webhook.go, plus tests) confirmed none
assert pointer identity across calls, so the return-semantics change is safe.

GraphService.FindByID already handles the resulting !IsNodesMetadataPopulated() case by
repopulating (internal/services/graph_service.go:69-74) — the same cost the Postgres driver
already pays on every call, not a regression.

Proof — the F-01.3 tests go from failing to passing

These were checked in on main as part of F-01 verification and are this task's stage-1
evidence; not touched by this PR:

• internal/services/f01_3_verification_test.go (3 unit tests)
• tests/functional/f01_3_verification_test.go (memory contract)
• tests/functional/f01_3_verification_pg_test.go (Postgres contract, //go:build functional)

All 4 confirmed red on this branch before the fix, green after.

Test plan

• make swagger && make lint && make build && make test — lint 0 issues, build clean,
  make test 736/738 passing. The 2 remaining failures
  (TestWorkflowFunc_HandleMessage_GivesNodeFunctionTheNodeSpanContext,
  TestWorkflowHandler_ResumeWithPendingSubWorkflow_KeepsExactlyOneDistinctChild) are
  F-01 findings 1 and 2 (A-04 / A-02 territory) — confirmed pre-existing on this branch
  without this fix via git stash, unrelated to graph_memory.go, out of scope for
  A-06.
• DB_POSTGRES_DSN=... make test-functional against a real Postgres 17 (make infra-up)
  — 140/140 passing, including PostgresGraphRepository live schema edit isolation contract — driver parity confirmed both ways.
• go test -race -count=1 ./internal/repositories/... ./internal/services/... ./tests/functional/... — clean.

🤖 Generated with Claude Code