Skip to content

Keep lineup drafts away from the agent they came from - #425

Merged
SunkenInTime merged 1 commit into
mainfrom
t3/lineup-draft-delete
Nov 1, 2026
Merged

SunkenInTime merged 1 commit into
mainfrom
t3/lineup-draft-delete

Conversation

@SunkenInTime

@SunkenInTime SunkenInTime commented Nov 1, 2026 •

Copy link
Copy Markdown
Owner

Same steps on both builds. After Cancel the agent is gone without the fix and still there with it

The steps: hover the draft, press E, press Cancel

A user building a lineup from a placed agent could lose that agent without seeing it happen. Right-click an agent, choose Create Lineup, hover the draft and press E, and the placed agent is deleted. The screen doesn't change, because the draft is drawn in the same spot, so you find out when you press Cancel and the agent isn't there.

ci: test analyze

What was wrong

The draft was the placed agent: same object, same id. I reproduced all of this on main in a Windows release build:

On the draft Before
Hover and press E Placed agent deleted. In a cloud strategy the delete reached the dev server, remote items 1 to 0
Drag onto the trash Placed agent deleted
Drag it anywhere Placed agent moved, with no undo step and no cloud op, and it stayed moved after Cancel. A drop on the right panel put it off the map at (1794, 26)

Undo brought the agent back after both deletes, if you noticed. It did nothing for the move, and a cloud strategy kept showing synced with the old position on the server.

The Delete key was never affected. It only acts on a selection and is ignored in lineup mode. E is the shortcut for the hovered item, and it's the key printed on the trash tile.

The fix

Two small changes, and both are needed:

  • setDraftAgent stores a copy with the same id, so moving a draft moves only the draft.
  • _removePlacedAgent in delete_helpers.dart refuses while a lineup mode is on. Both delete paths use it. A lineup mode keeps the page out of reach, so an agent acted on there is a draft.

I started with the second change only. The running build then showed the trash drop leaving the agent alive but dragged under the tile, with no undo and the server out of date. That's worse than the delete it replaced, hence the copy.

E on a draft now does nothing, the same as E on a draft dragged from the sidebar. Cancel is still how you drop a draft.

Lineup ids and the saved format don't change. The draft keeps the agent's id, which is why a saved lineup's origin agent still does too. agent_widget.dart is untouched, to keep the rebase of #417 small.

Ground truth

Windows release driver builds against the dev deployment, new script tool/drive/scripts/lineup-draft-delete.json. It first deletes a plain hovered agent with E, which proves keys are landing, then runs the draft steps. Both modes finished CLEAN on the fixed build:

  • Local: E, Delete and a trash drop on the draft, then Cancel. One agent left at its placed position (901.3, 425.5).
  • Cloud: same steps with a second script. Remote items stayed at 1, queue empty, synced.

test/lineup_draft_delete_test.dart has the shortcut and off-map drop cases. Both failed before the fix, with the agent gone. 145 tests across the lineup, selection and delete files pass.

Not covered

  • I didn't run this on web. The code is shared Dart and has no web branch.
  • A draft dropped on the trash still stays on the map at the drop spot, as a sidebar draft already does. Cancel clears it.
  • Driver keys get dropped when another window takes focus on a shared desktop, so runs needed retries. That's why the script starts with the plain-agent check: a dropped key fails the run instead of passing it.

RetriggerView in GreptileConfidence Score: 5/5

The PR appears safe to merge; no actionable issues were found.

Summary

The PR separates lineup drafts from their source agents and prevents draft delete gestures from deleting those agents.

  • Lineup drafts can move or be deleted without changing their placed agent.

No actionable issues found. Source checks support both parts of the fix; tests and Windows builds were not rerun during this review.

SunkenInTime explicitly acknowledged that trash drops leave the draft at the drop spot, web was not tested, and driver runs may need retries when focus changes. These are not reported as findings.

Reviews (1) · Last reviewed commit: "Keep lineup drafts away from the agent t..." · Reviewed by Greptile

A draft from Create Lineup was the placed agent itself: same object, same
id. The delete shortcut or a drop on the trash deleted the placed agent,
and dragging the draft moved it with no undo step or cloud op.

The draft is now a copy with the same id, and nothing deletes a placed
agent while a lineup mode has the page out of reach. Lineup ids and the
saved format don't change.
@coderabbitai

coderabbitai Bot commented Nov 1, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 27 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 9ffb2063-66cc-40c3-a1b4-839dad7d6786
Commits

Reviewing files that changed from the base of the PR and between 9e799b1 and c7e1a81.

Files selected for processing: 5
  • lib/const/line_provider.dart
  • lib/dev/drive/drive.dart
  • lib/widgets/delete_helpers.dart
  • test/lineup_draft_delete_test.dart
  • tool/drive/scripts/lineup-draft-delete.json
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant