feat: #2288 workflow-scoped key-value state (State Get + State Set) - #2368
feat: #2288 workflow-scoped key-value state (State Get + State Set)#2368dickwin2003 wants to merge 1 commit into
Conversation
…te Set)
Adds a per-workflow key-value store that survives a run, so monitor
workflows can keep a cursor ("last block scanned", "already alerted on")
instead of re-scanning a rolling window or provisioning an external
database for one value.
- workflow_state table: unique (organization_id, workflow_id, key),
jsonb value, per-write version, nullable expires_at, cascading FKs;
migration 0152 generated with db:generate
- State Get / State Set system actions dispatched like Database Query,
classified egress: none, and listed in the MCP schema catalog
- Scope comes from the execution context, never config (the
circuit-breaker rule), so cross-workflow and cross-org access have no
path
- state/set is an atomic upsert and accepts expectedVersion for
compare-and-set; get returns the version to feed it
- Enforced limits: 8 KB serialized value, 100 keys per workflow, ttl
clamped to 365 days; expired rows are evicted on touch
- Builder UI: palette entries and config forms for both actions
- Not in the direct-execute node resolver or codegen export: standalone
node execution has no workflow scope, and exported bundles cannot
reach this table (same treatment as the circuit-breaker actions)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
About the
|
suisuss
left a comment
There was a problem hiding this comment.
Welcome, and thanks for a diff that arrives with its registration complete - CONTRIBUTING.md and plugins/AGENTS.md carry the conventions, and the four system-action registration points are all here, which is the part most first contributions miss.
What this changes
A workflow_state table (drizzle/0152_tired_silver_sable.sql, model at lib/db/schema.ts), keyed unique on (organization_id, workflow_id, key) with version, expires_at and updated_by_execution_id, cascading from both organization and workflows. A store at lib/workflow/nodes/workflow-state/store.ts with the limits, the pure helpers, and a transactional setWorkflowStateValue that does compare-and-set when expectedVersion is given and an upsert otherwise, gating the 100-key ceiling only when no row exists. Two "use step" wrappers with maxRetries = 0 that take their scope only from input._context. Registration in all four places - SYSTEM_ACTION_TYPES, the executor dispatch table, system-action-capabilities.ts as egress none, and the MCP catalogue. Then the builder surfaces: two entries in the System group and two config forms.
I diffed 0152_snapshot.json structurally against staging's 0151: prevId chains correctly, 72 to 73 tables, and the only delta is public.workflow_state, matching the SQL and the model exactly. The migration is additive and safe - CREATE TABLE locks nothing, the two ADD CONSTRAINT take SHARE ROW EXCLUSIVE on organization and workflows but validate instantaneously against an empty child, and the deployed migrator sets lock_timeout=60000 so it fails rather than queues. Both indexes are built in the same migration on an empty table, so this correctly needs no @requires-db-prep.
Does it match the description
Undersells. The store's own comment and the MCP catalogue both say numbers and booleans are stored as-is, and through the editor they are not - see the fourth blocker.
Blocking
-
components/workflow/config/action-config.tsx:745-758- the expectedVersion field cannot accept what its own help text tells you to put in it. The help says "Pass the version returned by State Get (as @ reference)", andonChangerunse.target.value.replace(/[^0-9]/g, ""). -> Paste{{@state-get-1:State Get.version}}and the field stores"1", the digit out of the node id. Every run then compares against version 1, so the second write to that key ever fails with "expected version 1, current 2", permanently, with nothing to indicate why. The runtime side is fine -resolveExpectedVersionaccepts numeric strings and the template resolves - so this is the sanitizer defeating the documented pattern. Same sanitizer on TTL at:723-726. -
lib/workflow/nodes/state-get/step.ts:58- a miss returns{success: true, exists: false, value: null}with noversionkey at all. The executor stores the step result directly as the node's output (executor.workflow.ts:3523-3525), so a downstream{{@get:State Get.version}}resolves to nothing,recordUnresolvedfires withno-path, andassertResolvedthrows. -> The first execution of the documented State Get then State Set cursor fails before it can create the key. Returnversion: nullon a miss, or say in the catalogue that the field is absent and the config must tolerate it. -
lib/workflow/editor/template-helpers.ts:209-224andcomponents/ui/template-autocomplete.tsx:130-148- neither learned about the new actions, so both fall throughfindActionById(undefined for a system action) to the terminal default[{ field: "data" }]. -> The only field the@menu offers for a State node is{{@state-get-1:State Get.data}}, the step result has nodatawrapper, and the run aborts with the sameTemplateResolutionError. The discoverable path through the UI is a guaranteed failure. Both files special-caseFor EachandCollectalready; add the two here. -
lib/workflow/nodes/workflow-state/store.ts:143-163- "numbers and booleans stored as-is" holds for MCP and not for the editor.processTemplatesoperates on string fields andformatConfigValuestringifies, sovalue: "{{@trigger:Trigger.blockNumber}}"reaches the step as"4219", andcoerceStateValueonly JSON-parses strings starting with{or[. -> It is stored as the JSON string"4219", and a downstream Condition comparing it against the same trigger field compares lexically:"100" < "99"is true. Objects and arrays round-trip; scalars do not, and the two authoring paths disagree on the stored type of the same workflow. -> Either coerce a bare numeric or boolean string incoerceStateValue, or correct the claim in the store comment, the config help text and the MCP catalogue.
Mechanical - actionable as-is
-
components/workflow/config/action-config.tsx:653-666- the Key field is a plain<Input>while its help text says "Use @ to build the key from previous node values". Every other template-capable text field in the file usesTemplateBadgeInput(:208,:414,:482). Raw{{…}}still resolves at runtime, so this degrades rather than breaks, but the affordance the help promises is not wired. -
lib/workflow/nodes/workflow-state/store.ts:410- overwriting an expired row takes theexistingbranch and returnscreated: false, immediately afterState Getreportedexists: false. The catalogue definescreatedas "created the key rather than overwriting it". -
lib/workflow/nodes/state-set/step.ts:62- the guard rejects onlyundefined, soconfig.valueof""from the editor sets the key to the empty string, and an MCP-authored"value": nullreaches an insert against anotNull()jsonb column. I could not verify from the repo whether drizzle's jsonb mapper writes JSON'null'or SQL NULL there - mark that one UNSURE - but screening both is one line. -
drizzle/0152_tired_silver_sable.sql- the last thirteen migrations are descriptive (0151_keep_1328_org_circuit_breaker.sql); this one keeps drizzle's generated codename. -
idx_workflow_state_expires_atindexes a column nothing sweeps. Eviction is lazy-on-touch only, so an abandoned expired key is invisible to reads and to the 100-key count and occupies storage indefinitely, while the index costs write amplification for a sweeper that does not exist. -
components/workflow/nodes/action-node.tsx:134-166-getProviderLogohas no case for the new actions, so both render the amberZapfallback, identical to HTTP Request. -
Screenshots: the System group of the action picker with both new entries; the State Set config form with Value, TTL and expectedVersion filled; and the State Get form. Those three are what the diff renders.
With the team
-
Whether a lost compare-and-set should be recoverable.
stateSetStep.maxRetries = 0plus a CAS that fails the node means the "re-read with State Get and retry" pattern the catalogue describes is not expressible - there is no loop construct in the builder, so a lost race fails the whole run. I'm weighing allowing a retry on the step against adding a first-class retry-on-conflict output the author can branch on; the tradeoff is an unbounded retry on a write that moves state against a config surface nobody has asked for yet. I'm taking it to the core team and will come back. -
Whether an org-level ceiling is needed. The only cap is per workflow, every plan has
maxWorkflows: -1, and there is no reaper. I'm weighing a per-org key or byte budget against leaving it uncapped until it matters.
Does it match the description: Scope creep
One clean seam. components/workflow/config/action-config.tsx and action-grid.tsx on one side, everything else on the other. The engine half ships and is correct with the UI reverted - the actions stay usable from MCP, the executor dispatches them, egress and validation are right, they are simply absent from the palette. The UI half does not survive alone: it uses bare string literals so it compiles, but validateWorkflowActionConfigs skips only members of SYSTEM_ACTION_TYPES, so a State node fails save with UNKNOWN_ACTION_TYPE. That is a one-way dependency, so the split is available but not required, and everything inside the engine half is genuinely atomic - the union, the egress record and the dispatch table are compile-time coupled, and the schema fails at first execution without the migration. Not scope creep; I am recording it so it is not re-litigated.
Verdict
Changes requested - the version field strips the reference it asks for, a State Get miss omits the field the documented pattern reads, and the only output the @ menu offers for these nodes aborts the run.
Closes #2288 (part of #2293). Also my submission for the DoraHacks "KeeperHub - The Agent Economy" hackathon bounty track, held to the normal merge bar.
Storage: Postgres, not Redis
Argued from the requirements as the issue asks:
lib/redis.tsdocuments itself as "never a source of truth" (null client whenREDIS_HOSTis unset, fail-fast commands, loss tolerated). A monitor cursor is exactly the value whose loss is visible - the resolved-alert-reopens failure the issue describes - so it belongs in the durable store the app already operates. Bonus: self-hosted installs need zero new infrastructure, and the user provisions nothing, which is the "serverless-first" spirit of the ticket.expires_at(timestamptz, null = no TTL) filtered on read, expired rows deleted on touch. No background job is required for correctness, and an indexedexpires_atkeeps a future sweep cheap if one is wanted.updated_by_execution_idalready answers "which run wrote this cursor" from day one.What ships
lib/db/schema.ts+ migration 0152):workflow_statewith unique(organization_id, workflow_id, key),value jsonb,version(bumped on every write), nullableexpires_at,updated_by_execution_id; FKs toorganizationandworkflowswithON DELETE CASCADE. Migration generated withpnpm db:generateper AGENTS.md.State GetandState Set, registered the compile-enforced way: entries inSYSTEM_ACTION_TYPES, the executor'sSYSTEM_ACTIONSdispatch table,SYSTEM_ACTION_EGRESS(bothnone- in-process DB access, no user-chosen destination, same as the circuit breakers), andlib/mcp/workflow-schema-constants.tsso they surface invalidate_workflow/ the schemas route. Builder UI: palette entries plus config forms (key, value editor, ttl, advancedexpectedVersionfield).(organizationId, workflowId)from_contextand never from config - the circuit-breaker steps' rule. No API surface takes a workflow id, so cross-workflow and cross-org access have no path. This also answers the direct-execution question:/api/execute/nodehas no workflow scope, so the actions are deliberately not inaction-resolver.tsand fail with a clear error if run without workflow context.state/setis an atomic upsert (update-then-insert inside a transaction, withON CONFLICT DO UPDATEcovering a concurrent insert of the same key). WithexpectedVersionit is a compare-and-set: on mismatch the step fails with a structured conflict error naming expected vs current version instead of silently losing the race.state/getreturnsversionto feed it. Both steps setmaxRetries = 0so the durability layer never replays a bump.ttlclamped to [1s, 365 days]. The ceiling only gates writes that create a new row; overwriting an existing key (live or expired) never grows the store. An expired key frees its slot.The actions are deliberately not added to the codegen export map (same treatment the circuit-breaker actions shipped with): an exported standalone bundle cannot reach this org table.
Testing
none+ not plan-gated + validator acceptance), mirroring the circuit-breaker suites.tests/unit/validate-workflow-structural.test.tspinned key list extended 7 -> 9.pnpm type-check,pnpm type-check:executor,pnpm fix(biome) clean, andpnpm buildpasses locally (against a.env.localand a socket-served embedded Postgres for static generation).pnpm test:unitrun: 22,030 passed; the 142 failures all sit in 5 files (code-sandbox runtime, docs markdown, a Windows path-sensitive plugins walk, one slow-import timeout) and were verified to fail identically on the untouched base commit via a clean worktree run, so none are introduced here.Addresses the design comment on the issue: storage question resolved as above; open question 2 resolved as system action types (that is what surfaces them in the builder/MCP schema with no extra wiring); question 3 resolved by adopting the proposed 8 KB / 100 keys ballparks.