Skip to content

Promote from the editor - #5005

Open
elias-ba wants to merge 8 commits into
4859-edit-in-sandboxfrom
4929-promote-from-editor
Open

Promote from the editor#5005
elias-ba wants to merge 8 commits into
4859-edit-in-sandboxfrom
4929-promote-from-editor

Conversation

@elias-ba

@elias-ba elias-ba commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Description

This PR adds promoting a workflow from the sandbox editor back into its parent project, along with the lifecycle controls needed to make that safe. Promote merges the workflow you are editing into the parent's live workflow, then offers to archive the sandbox or keep it, so you can promote several related workflows before retiring it. To support editing and testing safely, a non-live workflow (a draft, or anything in a sandbox) can have a trigger turned on for testing behind a warning, and read-only (live) workflows now consistently hide run and edit actions instead of showing them disabled. It also includes a pass over the header, the confirmation dialogs, and the edit-in-sandbox picker.

Closes #4929

Validation steps

  1. Open a live workflow on the main project. The header shows a "Live" badge (with an explanatory tooltip) next to the version picker, no Save or Run buttons, and "Edit in sandbox" / "Switch to draft" actions. You can still read the workflow's configuration and copy the webhook URL, but the canvas and inspectors are read-only and undo/redo does nothing.
  2. Click "Edit in sandbox". The picker opens with a "Create a new sandbox" section (pressing Enter creates it; a duplicate name shows an inline error) and, when any exist, a "Join an active sandbox" list. Creating one lands you in the sandbox with the workflow as an editable draft and its trigger off.
  3. In the sandbox, select the trigger and turn it on. A warning appears about receiving real events (with a "don't show again" option); confirming enables it. Turning it off does not warn.
  4. Make a change and click "Promote". Confirm, then either "Keep sandbox" (you stay in the sandbox and can switch to another workflow and promote it too) or "Archive sandbox" (you return to the parent, now live with your changes, and the sandbox is scheduled for deletion). An editor who cannot archive sees only "Done".
  5. Confirm the parent workflow reflects the promoted changes and its version badge is current.

Additional notes for the reviewer

  1. This stacks on Edit in sandbox: create or join a sandbox from a live workflow #4901 (edit in sandbox) and targets that branch, so it is best reviewed after Edit in sandbox: create or join a sandbox from a live workflow #4901. The user-facing copy in the new dialogs, the badge tooltip, and the picker is still being finalised, which is why this is a draft.
  2. On import, a brand-new workflow is labelled live or draft based on whether any trigger is enabled, while an existing workflow keeps its stored state on a round-trip, so a GitHub or CLI sync cannot silently flip a draft whose trigger you turned on for testing. Carrying draft/live explicitly in the wire format is tracked separately (Carry workflow state in the wire format (project.yaml + CLI) #4897).

AI Usage

Please disclose whether you've used AI anywhere in this PR (it's cool, we just want to know!):

  • I have used Claude Code
  • I have used another model
  • I have not used AI

You can read more details in our Responsible AI Policy

Pre-submission checklist

  • I have performed an AI review of my code (we recommend using /review
    with Claude Code)
  • I have implemented and tested all related authorization policies.
    (e.g., :owner, :admin, :editor, :viewer)
  • I have updated the changelog.
  • I have ticked a box in "AI usage" in this PR

elias-ba added 6 commits July 19, 2026 20:01
MergeProjects built each trigger with the kafka_configuration embed left as a
%KafkaConfiguration{} struct (stringify_keys only touches top-level keys), but
the provisioner import expects plain maps, so merging or promoting a workflow
with a Kafka trigger raised an Ecto.CastError. Serialize the config to a plain
map (the fields the import round-trips, credentials omitted), matching the
export path. Pre-existing: the sandbox-merge LiveView flow crashed the same way.
From a workflow open in a sandbox, a Promote action merges it back into the
parent using the existing sandbox merge, scoped to that workflow, then archives
the sandbox. Authorized by merge_sandbox (editor/admin/owner on the parent),
resolved server-side. A two-button confirm dialog; on success the user lands in
the parent project. The archive is best-effort and non-atomic, so a
merged-but-not-archived result is reported gracefully.

Part of #4929.
Save the sandbox before promoting (aborts if the save fails), so promote
reflects the current editor state; the confirmation now reads "Save and
promote". Add a "Promoting..." spinner state, and show the success toast on
the parent page after navigation (via a query param the editor reads on load)
so the hard navigation no longer wipes it.

Part of #4929.
A sandbox merge/promote (and the provisioner import path generally) writes the
workflow to the database but never refreshed the workflow's live collaborative
document, so anyone on that workflow saw stale content and a stale version
badge whenever its SharedDoc was still warm. Pre-existing: the sandbox-merge
LiveView had the same gap.

The provisioner import now broadcasts a reconcile request per affected workflow
after its transaction commits; the process that owns the live SharedDoc
subscribes and resets the document in place from the database (which also sets
lock_version, fixing the badge), reusing the lifecycle reset mechanism. The
Y.Doc mutation only runs in the owning process, so no transaction crosses a
process boundary. Merge and promote inherit this via the provisioner.

Part of #4929.
…g lifecycle controls

Promote now merges the current sandbox workflow into the parent and no longer archives the sandbox, so several related workflows can be promoted from one sandbox before it is retired. Archiving is a separate, explicit action gated on the delete-sandbox permission, which is also surfaced to the editor.

Add a way to enable a trigger on a non-live (draft or sandbox) workflow for testing without going live; it refuses live workflows and reuses the shared trigger-update path. On import, a brand-new workflow's draft/live state is inferred from its triggers (any enabled trigger means live), while an existing workflow keeps its stored state on a round-trip. Edit-in-sandbox brings the workflow in disabled, and a failed provision cleans up the half-built sandbox.

Part of #4929.
Add the two-phase promote dialog (confirm, then optionally archive the sandbox), keeping the user in the sandbox when they choose to keep it so they can promote another workflow. Restore the trigger enable/disable toggle in the trigger inspector footer for non-live workflows, guarded by a warning (with a 'don't show again' preference) before enabling one against real systems.

Harden read-only workflows: hide run-creation everywhere rather than showing it disabled, stop undo/redo from mutating a live workflow, and hide edit actions that were only rendered disabled. Polish the header (hide Save on read-only, move the lifecycle badge next to the version picker with a tooltip, drop the redundant read-only pill), restyle the confirmation dialogs, and refine the edit-in-sandbox picker (submit on Enter, inline duplicate-name error).

Part of #4929.
@github-project-automation github-project-automation Bot moved this to New Issues in Core Jul 21, 2026
@elias-ba
elias-ba marked this pull request as ready for review July 21, 2026 18:07
@elias-ba
elias-ba requested a review from lmac-1 July 21, 2026 18:07
@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown

Producing the review output.

Security Review ✅

  • S0 (project scoping): New promote, archive_sandbox, and set_trigger_enabled handlers in lib/lightning_web/channels/workflow_channel.ex derive project and workflow from join-time socket.assigns; parent is fetched server-side via fetch_parent_project/1, and set_trigger_enabled verifies the client-supplied trigger_id belongs to the loaded workflow.
  • S1 (authorization): promote gates on :sandboxes, :merge_sandbox against the resolved parent, archive_sandbox on :sandboxes, :delete_sandbox (owner/admin, with defense-in-depth re-check inside Sandboxes.schedule_sandbox_deletion/2), and set_trigger_enabled on authorize_content_edit (edit-workflow role + non-live state).
  • S2 (audit trail): Workflows.set_trigger_enabled/4 flows through save_workflow, which fires maybe_audit_workflow_state_changes on enable/disable transitions; promote merges via Provisioner.import_document which retains its existing audit_workflows + audit_channels steps.

@elias-ba
elias-ba marked this pull request as draft July 21, 2026 18:25
@elias-ba
elias-ba marked this pull request as ready for review July 21, 2026 18:26

@lmac-1 lmac-1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @elias-ba, thanks for this and sorry for the delay.

These are some issues that I think should be resolved, I'll let you decide if it should be follow on work:

  1. Clicking "Promote" on a sandbox workflow resets the live editor for every other workflow in the parent project, discarding anyone's unsaved changes (more details inline on provisioner.ex). I'd expect unsaved changes on the live promoted workflow to be discarded, but not other workflows that weren't promoted.
  2. Renaming a workflow via "Edit in sandbox" and then clicking "Promote" creates a new workflow rather than updating the existing one. This splits its history into a new workflow, and the new copy arrives as a draft with its trigger disabled - so it doesn't run. A new workflow also means new job and trigger ids.
  3. Question on maybe_infer_workflow_state/2: The new :loaded clause means an existing workflow always keeps its stored state. If a sync or CLI deploy pushes a document with every trigger disabled, does the workflow stay :live? If so, it'd be read-only in the editor but not processing anything. The draft-preservation goal in the description only seems to require guarding the draft → live direction. Is blocking live → draft intentional?
    Everything else outsideof this

I would also like @midigofrank to review this to double check I haven't missed anything from the provisioner side of things.

Low priority issues:
These shouldn't block merging and I am documenting them for completeness.

  1. Something strange happens when I enable/disable the trigger in a sandbox: it seems like it increases the version numbers in the version dropdown but the version number does not update. Example:
    1. Enable the trigger in a sandbox
    2. Version dropdown updates to v1 (Save button has no red unsaved indicator)
    3. Disable the trigger
    4. Version dropdown stays at v1
    5. If you enable/disable, you will see that version dropdown stays at v1 but if you open it you will see lots of different version numbers higher than 1
    6. Clicking Save updates to latest
  2. Promote button is active for a collaborator who does not have permission to promote - should it be read only? Example - a collaborator only with access to the sandbox but not production project can click Promote. They get a toast message telling them they don't have permission for that action, but wondering if the button should be read only with tooltip message?

read-only view, where "Live" (the current state) doesn't explain
why this view is read-only. */}
{!(
lifecycleState === 'live' &&

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is lifecycleState === 'live' && !isNewWorkflow && !isSandbox the condition for the live badge being visible? If so, I would group this together into a variable like const showsLiveBadge = lifecycleState === 'live' && !isNewWorkflow && !isSandbox; so it's clearer for someone else reading the code later.

collaborative document should reconcile against, excluding workflows the
document marks for deletion. Only workflows present in the document are
returned, so sibling workflows on the target are never touched.
"""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This promise doesn't quite hold on the scoped promote path:

Only workflows present in the document are returned, so sibling workflows on the target are never touched.

When selected_workflow_ids is set, build_merged_workflows/6 emits build_passthrough_workflow/1 for every unselected target workflow (merge_projects.ex:853-859). Those entries carry a plain "id" and no "delete", so they survive the filter here and each gets a ReconcileRequested.

The net effect is that promoting workflow A resets the live document for every other workflow in the parent project. reset_shared_doc/2 clears jobs, edges, triggers and positions, then re-serialises from the database, so anyone editing an untouched sibling workflow loses their unsaved canvas changes.

Nothing in the database is affected—IDs, runs and logs are all fine—but it's a genuine "my work just vanished" experience for anyone with the parent project open during a promote.

To reproduce:

  1. Have a project with 2 workflows (one live, one draft)
  2. Open workflow A in one tab and make a change, keep the tab open
  3. Open workflow B in a second tab and click "Edit in sandbox".
  4. Make a change in the sandbox and click Promote
  5. The changes in step 2 are wiped even though it's a different workflow

# (Session.clear_and_reset_doc / Persistence.clear_and_reset_workflow). All
# Yex collections are retrieved before the clear transaction to avoid a VM
# deadlock, and serialize_to_ydoc runs its own transaction afterwards.
defp reset_shared_doc(shared_doc_pid, workflow) do

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From Claude (non-blocking but nice to tidy up):

reset_shared_doc/2 plus its private clear_array/1 and clear_map/1 are a verbatim third copy of Session.clear_and_reset_doc/2 and its helpers (session.ex:598-639) — same pre-transaction handle retrieval, same clear transaction, same WorkflowSerializer.serialize_to_ydoc/2 call. Persistence.clear_and_reset_workflow/2 (persistence.ex:130-160) is a partial second copy with its own clear_array/1.

@midigofrank
midigofrank self-requested a review July 28, 2026 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: New Issues

Development

Successfully merging this pull request may close these issues.

2 participants