Skip to content

Sandbox merge no longer deletes a parent workflow added after the fork#4940

Open
elias-ba wants to merge 1 commit into
mainfrom
4919-merge-deletes-parent-workflow
Open

Sandbox merge no longer deletes a parent workflow added after the fork#4940
elias-ba wants to merge 1 commit into
mainfrom
4919-merge-deletes-parent-workflow

Conversation

@elias-ba

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

Copy link
Copy Markdown
Contributor

Description

This PR fixes a data-loss bug in the sandbox merge. A workflow added to a project after a sandbox was branched was flagged for deletion and pre-selected on the merge screen, so a routine merge could silently delete it. Workflows present in the project but not in the sandbox now default to unchecked, so deletion is opt-in and anything left unchecked is kept. Those rows are also labelled honestly, distinguishing a workflow added after the fork from one that existed but is absent from the sandbox. The merge logic itself is unchanged; this only touches the merge screen.

Closes #4919

Validation steps

  1. Create a sandbox from a project, then add a new workflow to the project.
  2. Merge the sandbox back. The newly added workflow appears in the merge screen unchecked, labelled as added after the sandbox was created, and is kept after the merge.
  3. Explicitly check a project-only workflow and confirm it is still deleted on merge.

Additional notes for the reviewer

The merge logic is unchanged; this only affects the merge screen's defaults and which workflows are selected.

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
  • I have updated the changelog.
  • I have ticked a box in "AI usage" in this PR

@github-project-automation github-project-automation Bot moved this to New Issues in Core Jul 6, 2026
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown

The diff is confined to sandbox merge UX changes: relabeling deletion states, defaulting target-only workflows to unchecked (safer), and adding an added_after_fork flag. Authorization gates (Permissions.can?(:sandboxes, :merge_sandbox, ...)) and project scoping via preloaded source/target projects are unchanged.

Security Review ✅

  • S0 (project scoping): MergeWorkflow entries are built from Repo.preload(:workflows) on the already-scoped source/target projects reached via authenticated workspace_projects; no new user-supplied project_id paths introduced (index.ex:870-921).
  • S1 (authorization): N/A — no new LiveView events or handlers; the pre-existing Permissions.can?(:sandboxes, :merge_sandbox, actor, target) gate at index.ex:457 still guards the merge, and the diff only reshapes the selected-ids payload passed to perform_merge.
  • S2 (audit trail): N/A — perform_merge and downstream write paths are untouched; the change only alters which ids are handed in, and the new default (unchecked target-only workflows) makes silent deletions less likely, not more.

A workflow added to a project after a sandbox was branched was flagged for deletion and pre-selected on the sandbox merge screen, so a routine merge could silently delete it. Workflows present in the project but not the sandbox now default to unchecked (deletion is opt-in), the merge no longer takes the merge-everything-and-delete-unmatched path, and the merge screen labels those rows honestly instead of "Deleted in sandbox".
@elias-ba elias-ba force-pushed the 4919-merge-deletes-parent-workflow branch from c3f4cfb to d881a42 Compare July 6, 2026 11:10
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.

Sandbox merge deletes a parent workflow added after the sandbox was forked

1 participant