Skip to content

Keep a lineup draft from acting on the agent it came from - #446

Merged
SunkenInTime merged 3 commits into
mainfrom
t3/draft-menu-identity
Nov 1, 2026
Merged

SunkenInTime merged 3 commits into
mainfrom
t3/draft-menu-identity

Conversation

@SunkenInTime

@SunkenInTime SunkenInTime commented Nov 1, 2026 •

Copy link
Copy Markdown
Owner

Draft menu and long press, before and after, on the desktop driver build

Same check in Edge on the web build

Real frames from the driver runs, stepped at 1.5 s each:

Long press on a draft, then Cancel

ci: test

Someone building a lineup thinks they're editing the draft. Before this PR, holding the mouse on the draft marked the placed agent underneath as dead, and they only saw it after Cancel, with a red X on an agent they never touched. The draft's right-click menu also offered Create Lineup and Copy to next or previous page, and both acted on the placed agent.

#425 gave a draft its own copy of the agent but kept the same id, on purpose, because lineup ids can't change. AgentWidget looked agents up by that id, so a draft found the placed agent and offered its actions. Now a draft never does that lookup. Its menu is the ability row and Weapon, and a long press on it does nothing.

Removing the lookup also makes the onWeaponSelected == null guard from #417 on Delete unnecessary, so that's gone. A placed agent's menu and long press are unchanged, and a test pins the long press.

What I saw in a running build

Windows release driver builds against the dev deployment. The only difference between the two builds is this fix.

  • lineup-draft-identity.json, new: fails 4 steps on the build without the fix (placed agent dead, Create Lineup and Copy in the draft menu) and passes all 43 with it. It starts by long-pressing a plain agent twice, so a dead long press can't pass by accident.
  • lineup-draft-delete.json from Keep lineup drafts away from the agent they came from #425: clean.
  • right-click-local.json from Right-click a page to rename or delete it, and an agent to delete it #417: clean on an earlier build of this branch, before I removed a hover change that turned out to be unneeded.
  • Edge at 1600 x 900 on a web build against dev, both builds, same steps: the second picture above.

I reran the new script and right-click-local.json on a rebuild of the merged head, and both were clean. CI passed on the same commit.

Not covered

  • A draft made by dragging an agent from the sidebar has a fresh id, so it never had this bug. I didn't drive it.
  • I checked whether a vision cone can reach a draft. It can't, because starting a lineup closes the cone tools and opening them ends the lineup.

The driver gained expectDead and a hold on tapAt for a long press. Both are in lib/dev/drive, which only compiles into driver builds.

RetriggerView in GreptileConfidence Score: 5/5

The PR appears safe to merge.

Summary

This PR stops lineup drafts from looking up and changing the placed agent they came from.

  • Lineup drafts no longer use actions meant for their source agents.

No actionable issues or rule violations were found. Caller checks confirm that active drafts supply the callback used to identify them. Searches found no matching uses in the three related repositories.

This review checked source code only; it did not rerun the tests or capture the UI. The running-build results are supplied by SunkenInTime in the PR description. No further code change is recommended from this review.

Reviews (1) · Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." · Reviewed by Greptile

@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 13 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: 8a10838f-fea6-4179-93ab-09429c502192
Commits

Reviewing files that changed from the base of the PR and between eb888f7 and 5a7b488.

Files selected for processing: 4
  • lib/dev/drive/drive.dart
  • lib/widgets/draggable_widgets/agents/agent_widget.dart
  • test/lineup_draft_delete_test.dart
  • tool/drive/scripts/lineup-draft-identity.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

@SunkenInTime
SunkenInTime merged commit f7e7c95 into main Nov 1, 2026
15 checks passed
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