OpenFn / OpenFn/lightning

Model "a workflow is being created" once, not four times

Open
#5,031 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Elixir
Stars
296
Forks
86
Avg merge
1d 13h
Merged PRs (30d)
50

Description

Depends on #4848

User story

As someone creating my first workflow, I want the four ways of starting one to
behave as four ways of doing the same thing, so changing my mind halfway through
doesn't destroy what was already being created.

Details

Four creation paths on /w/new, four separate "am I creating?" flags, none aware
of the others:

Path Flag Where
Build from scratch useActionLock CollaborativeEditor.tsx:197
Template select useActionLock TemplateBrowserModalWrapper.tsx:86
YAML import importState === 'importing' YAMLImportModal.tsx:62
Build with AI refs in the apply hook useAIWorkflowApplications.ts:206

Each lock stops its own path re-entering. Nothing stops a second path starting
while a first is in flight. All four write the same Y.Doc, then save it.

The sharp edge is scratch-then-AI. onBuildWithAI
(CollaborativeEditor.tsx:215-218) runs synchronously:

onBuildWithAI={(prompt: string) => {
  dismissLandingScreen();
  openAIAssistantPanel(prompt);
}}

which trips the effect at WorkflowEditor.tsx:245-267:

if (aiPanelJustOpened) {
  clearCanvas();          // :263 — knows nothing about an in-flight save
}

So the document is emptied underneath a save that hasn't returned. Depending on
the interleaving you get a persisted blank workflow, or a saved workflow that
doesn't match the canvas.

The landing screen guards one of the four: isBuildingFromScratch disables the
scratch card (LandingScreen.tsx:145). The AI textarea, the AI submit button,
the templates card and the YAML import link all stay live.

Disclosed at #5006 PR body L127. Not a regression from it.

On the duplicate-INSERT framing in the review — worth carrying into the
design, but the database is better defended than it looks. Saves are a
GenServer.call on the Session process (session.ex:238), so they queue rather
than interleave, and each one re-resolves through WorkflowResolver (:331)
rather than trusting a cached value, so a second save finds the row the first one
inserted (that's #4830). The damage here is a canvas and a database row that
disagree, not duplicate workflows. Design against the shared-document clobbering.

Row 10's residual belongs here. Cmd+\ over the template browser used to
stack two dialogs, each registering Escape at priority 100, so which one closed
came down to registration order. That case is fixed — the two modals now close
each other in createUIStore.ts. But that's a pairwise rule between two modals,
not the general one. Nothing stops a modal opening over an in-flight creation, or
being dismissed with Escape mid-save.

How to see it

The save is normally too fast to interleave with. Force the window:

  1. In workflow_channel.ex, add Process.sleep(5_000) at the top of
    handle_in("save_workflow", ...) (:348).
  2. Load /w/new and click Build from scratch.
  3. Within the 5s, type a prompt into the AI box and press Build it.
  4. The canvas empties. Check the database — the row that was saved does not match
    what the editor now shows.

Same trick reproduces the other pairings: open the YAML modal or pick a template
mid-save and watch neither path notice the other.

Implementation notes

Patch (~30 min), if the full change can't be scheduled now.
isBuildingFromScratch is already computed and already passed to
LandingScreen. Use it to disable the AI prompt and button as well as the
scratch card. That closes the one verified data-loss path and nothing else — five
other pairings stay open, so this is a stopgap, not the fix.

Real fix. One creationInFlight boolean in createUIStore, set by all four
paths and read by all four entry points:

  • useActionLock callers set it around their existing lock.
  • YAMLImportModal sets it when importState becomes 'importing'.
  • The AI apply hook sets it around handleApplyWorkflow on a new workflow.

Then: every landing-screen affordance disables while it's true; both modals
refuse Escape and close-button dismissal while it's true; the clearCanvas()
effect checks it before firing.

The AI path is the fiddly one — its flag isn't a boolean today, it's refs keyed
by message id (inFlightApplyRef, pendingReconnectApplyRef). Lift the "is a
creation running" question out without disturbing the streaming-apply dedup,
which uses the same refs for a different purpose.

Release notes

Fixed a bug where starting a second way of creating a workflow, while the first
was still saving, could leave you with an empty canvas.

User acceptance criteria
  • With the 5s sleep in place: Build from scratch, then submit an AI prompt — the
    canvas is not emptied, and the persisted workflow matches what's on screen.
  • While any creation is in flight, the other three entry points are disabled.
  • Neither modal can be dismissed, Escape included, while its own creation is in
    flight.
  • A regression test covers the scratch-then-AI interleaving.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with createUIStore and the four entry points named in CollaborativeEditor.tsx, TemplateBrowserModalWrapper.tsx, YAMLImportModal.tsx, and useAIWorkflowApplications.ts; trace how WorkflowEditor.tsx handles clearCanvas(). Reproduce the race using the 5-second sleep in workflow_channel.ex, then add regression coverage for scratch-then-AI and verify every alternate entry point and modal dismissal is blocked during creation.

Written by the indexing model from the issue text.

Assessment

Tech stack
elixir, typescript
Domain
backend, frontend, testing
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
65/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.