From dcf7372fbbe493528caf13c4b419905124fc89a4 Mon Sep 17 00:00:00 2001 From: Reynaldo Endis Date: Mon, 5 Oct 2026 18:56:47 +0000 Subject: [PATCH] fix: close remaining review items in runtime and Studio - contextMutex.Unlock panics on an unlocked mutex instead of blocking. - Metadata pack validation shares one Ajv compiler and evicts each schema from Ajv's identity cache, so in-place edits recompile and memory stays bounded by the LRU. - History equality uses JSON semantics (undefined equals absent), so no-op patches do not record undo steps; drops the fast-deep-equal dependency. - Remove the unused issue.duplicatedTransitionCondition message. --- CHANGELOG.md | 10 +++++++ experimental/runtime_control.go | 9 +++++- experimental/runtime_control_test.go | 12 ++++++++ studio/packages/editor-core/package.json | 3 +- .../__tests__/editorHistoryReducer.test.ts | 18 ++++++++++++ .../editor-core/src/i18n/messages/en.ts | 1 - .../editor-core/src/i18n/messages/es.ts | 1 - .../src/state/editorHistoryReducer.ts | 29 +++++++++++++++++-- .../editor-core/src/utils/metadataPacks.ts | 15 ++++++++-- studio/pnpm-lock.yaml | 3 -- 10 files changed, 88 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 761a6b1..b50f057 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Studio: metadata pack schemas are compiled with one shared Ajv instance instead of one instance per schema (128 distinct schemas: about 1.4 s to 60 ms). Edited schemas are still recompiled. +- Studio: `@rendis/statepro-studio-react` no longer depends on `fast-deep-equal`. + +### Fixed + +- Runtime: unlocking the machine mutex when it is not locked panics, like `sync.Mutex`, instead of blocking forever. +- Studio: an edit whose patch only sets absent fields to `undefined` no longer records an undo step that changes nothing. + ## [3.4.0] - 2026-10-05 ### Added diff --git a/experimental/runtime_control.go b/experimental/runtime_control.go index 806f03c..e5e8adb 100644 --- a/experimental/runtime_control.go +++ b/experimental/runtime_control.go @@ -21,7 +21,14 @@ func (m *contextMutex) init() { m.once.Do(func() { m.token = make(chan struct{}, 1); m.token <- struct{}{} }) } func (m *contextMutex) Lock() { m.init(); <-m.token } -func (m *contextMutex) Unlock() { m.token <- struct{}{} } +func (m *contextMutex) Unlock() { + select { + case m.token <- struct{}{}: + default: + // Match sync.Mutex: unlocking an unlocked mutex is a bug, not a deadlock. + panic("statepro: unlock of unlocked contextMutex") + } +} func (m *contextMutex) LockContext(ctx context.Context) error { if ctx == nil { return fmt.Errorf("context must not be nil") diff --git a/experimental/runtime_control_test.go b/experimental/runtime_control_test.go index b288ab3..d1462cb 100644 --- a/experimental/runtime_control_test.go +++ b/experimental/runtime_control_test.go @@ -468,3 +468,15 @@ func TestRuntime_TrackingLimitadoPreservaRollback(t *testing.T) { t.Fatalf("historial sin limite: %v", u.tracking) } } + +func TestRuntime_UnlockSinLockEntraEnPanicEnVezDeBloquear(t *testing.T) { + var mtx contextMutex + mtx.Lock() + mtx.Unlock() + defer func() { + if recover() == nil { + t.Fatal("se esperaba panic al desbloquear un mutex libre") + } + }() + mtx.Unlock() +} diff --git a/studio/packages/editor-core/package.json b/studio/packages/editor-core/package.json index 5242bea..28691d6 100644 --- a/studio/packages/editor-core/package.json +++ b/studio/packages/editor-core/package.json @@ -39,8 +39,7 @@ "ajv": "^8.17.1", "elkjs": "^0.11.0", "lucide-react": "^0.468.0", - "react-zoom-pan-pinch": "^3.7.0", - "fast-deep-equal": "^3.1.3" + "react-zoom-pan-pinch": "^3.7.0" }, "devDependencies": { "@stryker-mutator/core": "^10.0.0", diff --git a/studio/packages/editor-core/src/__tests__/editorHistoryReducer.test.ts b/studio/packages/editor-core/src/__tests__/editorHistoryReducer.test.ts index 7963b13..a2a7cec 100644 --- a/studio/packages/editor-core/src/__tests__/editorHistoryReducer.test.ts +++ b/studio/packages/editor-core/src/__tests__/editorHistoryReducer.test.ts @@ -154,6 +154,24 @@ describe("editorHistoryReducer", () => { expect(domainNext.present.isDirtyFromImport).toBe(true); }); + it("un patch con propiedades undefined que no cambia el JSON no crea un paso de historial", () => { + const initial = createInitialEditorHistoryState(createInitialEditorState()); + const node = initial.present.nodes[0]; + const next = editorHistoryReducer(initial, { + type: "apply-editor-action", + mode: "record", + action: { type: "update-node-data", payload: { nodeId: node.id, patch: { notAField: undefined } } }, + }); + expect(next.past).toHaveLength(0); + + const changed = editorHistoryReducer(initial, { + type: "apply-editor-action", + mode: "record", + action: { type: "update-node-data", payload: { nodeId: node.id, patch: { notAField: "value" } } }, + }); + expect(changed.past).toHaveLength(1); + }); + it("record apila en past y limpia future", () => { const initial = createInitialEditorHistoryState(createInitialEditorState()); const first = applyMachineId(initial, "machine-a", "record"); diff --git a/studio/packages/editor-core/src/i18n/messages/en.ts b/studio/packages/editor-core/src/i18n/messages/en.ts index db219c2..a53ee11 100644 --- a/studio/packages/editor-core/src/i18n/messages/en.ts +++ b/studio/packages/editor-core/src/i18n/messages/en.ts @@ -375,7 +375,6 @@ export const STUDIO_I18N_EN = { "issue.initialUnknownUniverse": "Unknown universe '{{universeId}}' in initials", "issue.initialUnknownReality": "Unknown reality '{{realityId}}' for universe '{{universeId}}' in initials", "issue.conditionMissingSrc": "Condition must have src", - "issue.duplicatedTransitionCondition": "Condition '{{src}}' is duplicated in transition conditions.", "issue.machineIdInvalid": "Machine id has invalid format", "issue.machineCanonicalInvalid": "Machine canonicalName has invalid format", "issue.universeKeyInvalid": "Universe key has invalid format", diff --git a/studio/packages/editor-core/src/i18n/messages/es.ts b/studio/packages/editor-core/src/i18n/messages/es.ts index 99cf9f5..4aad788 100644 --- a/studio/packages/editor-core/src/i18n/messages/es.ts +++ b/studio/packages/editor-core/src/i18n/messages/es.ts @@ -377,7 +377,6 @@ export const STUDIO_I18N_ES: Record = { "issue.initialUnknownUniverse": "Universo desconocido '{{universeId}}' en initials", "issue.initialUnknownReality": "Realidad desconocida '{{realityId}}' para universo '{{universeId}}' en initials", "issue.conditionMissingSrc": "Condition debe tener src", - "issue.duplicatedTransitionCondition": "La condicion '{{src}}' esta duplicada en conditions de la transicion.", "issue.machineIdInvalid": "Formato invalido para machine id", "issue.machineCanonicalInvalid": "Formato invalido para machine canonicalName", "issue.universeKeyInvalid": "Formato invalido para universe key", diff --git a/studio/packages/editor-core/src/state/editorHistoryReducer.ts b/studio/packages/editor-core/src/state/editorHistoryReducer.ts index c2c4aa5..88d0c32 100644 --- a/studio/packages/editor-core/src/state/editorHistoryReducer.ts +++ b/studio/packages/editor-core/src/state/editorHistoryReducer.ts @@ -12,7 +12,6 @@ import type { } from "../types"; import { editorReducer, type EditorAction } from "./editorReducer"; import { takeHistoryOwnership } from "./historyOwnership"; -import deepEqual from "fast-deep-equal"; export const HISTORY_LIMIT = 100; export const COALESCE_WINDOW_MS = 600; @@ -68,6 +67,32 @@ const toSnapshot = (state: EditorState): EditorHistorySnapshot => ({ isDirtyFromImport: state.isDirtyFromImport, }); +const definedKeys = (value: Record): string[] => + Object.keys(value).filter((key) => value[key] !== undefined); + +// Snapshot data is JSON. Compare it with JSON semantics: a property set to +// undefined equals a missing one, so a patch like { field: undefined } on an +// object without that field does not record an undo step that changes nothing. +const jsonEqual = (left: unknown, right: unknown): boolean => { + if (Object.is(left, right)) { + return true; + } + if (typeof left !== "object" || typeof right !== "object" || left === null || right === null) { + return false; + } + if (Array.isArray(left) || Array.isArray(right)) { + return Array.isArray(left) + && Array.isArray(right) + && left.length === right.length + && left.every((item, index) => jsonEqual(item, right[index])); + } + const leftRecord = left as Record; + const rightRecord = right as Record; + const leftKeys = definedKeys(leftRecord); + return leftKeys.length === definedKeys(rightRecord).length + && leftKeys.every((key) => jsonEqual(leftRecord[key], rightRecord[key])); +}; + const snapshotsEqual = ( left: EditorHistorySnapshot, right: EditorHistorySnapshot, @@ -78,7 +103,7 @@ const snapshotsEqual = ( // its own keys would miss a present imported model that must be cleared. const keys = new Set([...Object.keys(left), ...Object.keys(right)]); return (Array.from(keys) as Array).every( - (key) => Object.is(left[key], right[key]) || deepEqual(left[key], right[key]), + (key) => jsonEqual(left[key], right[key]), ); }; diff --git a/studio/packages/editor-core/src/utils/metadataPacks.ts b/studio/packages/editor-core/src/utils/metadataPacks.ts index 13df5a4..6f68c38 100644 --- a/studio/packages/editor-core/src/utils/metadataPacks.ts +++ b/studio/packages/editor-core/src/utils/metadataPacks.ts @@ -17,10 +17,16 @@ export const METADATA_PACK_MACHINE_ENTITY_REF = "machine"; const METADATA_SCOPES: MetadataScope[] = ["machine", "universe", "reality", "transition"]; -const createValidatorCompiler = () => new Ajv2020({ +// One shared compiler instead of a new Ajv instance per schema. addUsedSchema: +// false keeps compiled schemas out of the $id registry, so edited schemas can +// reuse a previous $id; validateBindingWithPack also drops each schema from +// Ajv's identity cache, so edits made in place recompile and memory stays +// bounded by validatorCache. +const validatorCompiler = new Ajv2020({ allErrors: true, strict: false, allowUnionTypes: true, + addUsedSchema: false, }); const VALIDATOR_CACHE_LIMIT = 128; @@ -698,8 +704,11 @@ export const validateBindingWithPack = ( const cacheKey = JSON.stringify(pack.schema); let validator = validatorCache.get(cacheKey); if (!validator) { - // Isolate $id registrations so edited schemas can reuse their previous ID. - validator = createValidatorCompiler().compile(pack.schema as object); + try { + validator = validatorCompiler.compile(pack.schema as object); + } finally { + validatorCompiler.removeSchema(pack.schema as object); + } } else { validatorCache.delete(cacheKey); } diff --git a/studio/pnpm-lock.yaml b/studio/pnpm-lock.yaml index f84f953..4c71d1b 100644 --- a/studio/pnpm-lock.yaml +++ b/studio/pnpm-lock.yaml @@ -109,9 +109,6 @@ importers: elkjs: specifier: ^0.11.0 version: 0.11.0 - fast-deep-equal: - specifier: ^3.1.3 - version: 3.1.3 lucide-react: specifier: ^0.468.0 version: 0.468.0(react@19.2.4)