mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
Waleed
8f85ded35f
`workflow_blocks.retry` is a jsonb column written verbatim. Three of its writers never validate what they store: the realtime batch-add and replace-state ops take untyped block records, and the admin/superuser import routes persist externally-authored workflow JSON. `load.ts` then asserted the blob was already a `BlockRetryConfig` and handed it straight to the HTTP boundary, where `workflowBlockStateSchema` bounds `maxTries` to 2..5 and `waitBetweenTriesMs` to 0..5000. That schema is shared between the PUT `/state` body, where the bound is right, and the GET `/api/workflows/[id]` and `/state` responses, where it is fatal. The response `.parse` in the shared route builder throws a ZodError, which is not an `OrchestrationError`, so the error policy declines it and it falls through to a 500. One out-of-range or partial stored value therefore made a workflow permanently unopenable, with no in-product repair — every UI write path reads the workflow first. The feature already declares clamp-on-read as its contract: the commit that added it says bounds are clamped on read rather than rejected, the TSDoc on `resolveBlockRetryConfig` says the same, and `block-retry.test.ts` asserts it. Execution has always honoured that. Only the read boundary disagreed, so that is what this fixes: the loader now constructs a real `BlockRetryConfig` from the blob through `normalizeBlockRetryTries` / `normalizeBlockRetryWaitMs`, filling defaults for missing fields and carrying `enabled` across unchanged. `loadWorkflowFromNormalizedTablesRaw` is the single read choke point for both apps — `@sim/workflow-persistence` for the Next app and the realtime server's full-state emit — so one edit repairs every reader, including rows that are already out of range, and the row self-heals on the next save. It matches `clampParallelBatchSize` a few lines below, which already pins a stored subflow value on the same path. Alternatives rejected: - Validating on write. It leaves every existing bad row fatal forever, and it would have to be repeated across three realtime ops plus roughly a dozen `saveWorkflowToNormalizedTables` callers, none of which share a validation seam. - Bounding `BlockRetrySchema` in `@sim/realtime-protocol`. Its own TSDoc is correct that batch-add and replace-state bypass it, so this closes one writer and leaves the 500. - Relaxing the response contract. It stops the 500 but leaves the editor rendering a number execution will never run. A test now pins the write bound so that shortcut fails loudly. - `resolveBlockRetryConfig`. It returns null for a disabled policy, which would erase the numbers a builder configured every time state is read. Tests: six cases in `packages/workflow-persistence/src/load.test.ts` (four red before this change) covering out-of-range enabled, out-of-range disabled with `enabled` preserved, missing fields, a non-boolean flag, an untouched in-range policy, and NULL meaning "runs once"; plus a contract test that the write bound still rejects out-of-range input.