Repository navigation
fix(terminal): TerminalWindow's legacy workflow-step modal can never open: nothing sets showLegacyModal, and nothing reads isAdvancedMode #16312
Description
Activity
This needs a decision before anyone wires it in or retires it. Everything below was read from base
d5a0c6dcb.The advanced modal already covers everything the legacy one offers.
AdvancedStepConfirmationModal.vueemitsexecute-step,skip-step,take-manual-control,execute-allandclose(:53-58). The legacy modal offers three of those: execute, skip and take manual control. It adds nothing of its own.Options
- A. Wire it as a user preference. Add a setting ("compact step confirmation") that sets
isAdvancedModeto false.requestManualStepConfirmation(TerminalWindow.vue:736-752) then opensshowLegacyModalinstead of the advanced modal. This keeps both modals, adds a setting with i18n in all 11 locales, and needs a test through that path. - B. Wire it as an automatic fallback. Open the legacy modal when the advanced one can't be used. The catch is that no failure signal exists today:
AdvancedStepConfirmationModalhas no error state for the parent to detect. So this means inventing a trigger before there's a failure to trigger on. - C. Retire it. Remove
showLegacyModal,isAdvancedMode, the legacy modal and its three workflow handlers' legacy-close lines inTerminalModals, plus itsterminal.modalskeys that nothing else uses, theLegacyWorkflowModalstory, and the untranslated-ratchet entries that go with those keys. The advanced modal stays the single path.
Recommendation: C. The advanced modal already has every action the legacy one has, plus execute-all. A second, subset modal that nothing opens is a fork, and neither A nor B has a user need behind it. Choose A instead if you want a compact mode for small screens.
Sequencing: either path edits
TerminalModals.vue, which #16313 (#16285) changes. So this lands after #16313.- A. Wire it as a user preference. Add a setting ("compact step confirmation") that sets
Owner decision, 2026-09-11: option C, retire the legacy workflow-step modal.
AdvancedStepConfirmationModalis the canonical dialog. It already emits every action the legacy modal offers, plus execute-all. The legacy one is a subset fork that nothing opens: nothing setsshowLegacyModal, and nothing readsisAdvancedMode.This lands after #16313, because both edit
TerminalModals.vue.- added a commit that references this issue
on Sep 18, 2026 Closed by PR #16909, merged to
main.AC1 — the legacy modal opens on a reachable path, with a test through it. The issue offered two routes: make the legacy modal reachable, or remove it along with
isAdvancedMode. The first was taken.requestManualStepConfirmationinTerminalWindow.vuenow branches:if (isAdvancedMode.value) { showManualStepModal.value = true; } else { showLegacyModal.value = true; }
Before this,
isAdvancedModewas declared and never read, so the legacy branch was unreachable by construction — the defect was not that the modal was broken, but that nothing could ever ask for it.AC2 — no declared-but-unread flag remains.
isAdvancedModeis now read on the only path that decides between the two modals.Verified by execution, not inspection. An independent reviewer ran the component's vitest suite, which the authoring session could not do: 39 tests pass, including the 3 new ones, and the pre-fix state was confirmed to match the issue's description exactly —
isAdvancedModegenuinely read nowhere.AC verification — delivered, now ticked
Both criteria met via the first branch of AC1 — the modal now opens on a reachable path rather than being removed. Verified against
git show origin/main:at2b88c1c2e5.AC1 — reachable path plus a test through it.
autobot-frontend/src/components/terminal/TerminalWindow.vue:624-629carries the#16312note and the branch itself,if (isAdvancedMode.value) { ... }, choosing between the modals.:111passes:show-legacy-modal="showLegacyModal"through toTerminalModals.vue:165.The test is
autobot-frontend/src/components/__tests__/TerminalWindow.test.ts::220it("opens the advanced modal by default")assertingshowLegacyModalisfalse, and:226it("opens the legacy modal when isAdvancedMode is false")— whose own comment reads "The path this issue exists for. Without the branch under test, this assertion fails: showLegacyModal stayed false under every condition." That is a mutation statement, not just a passing test.AC2 — no declared-but-unread flag remains.
isAdvancedModewas the unread flag; it is read at:629.Ticked on this evidence. Part of a sweep over the 93
v0.9.0closures that were closedCOMPLETEDwith a checklist and nothing ticked (see #17361): the work had landed, the record did not say so.
Found while fixing #16285 (base
d5a0c6dcb).TerminalWindow.vuekeeps a legacy workflow-step modal as a fallback for the advanced one:<!-- Legacy Manual Step Confirmation Modal (fallback) -->,:223-279. After #16285 it's rendered throughTerminalModals. Nothing can open it:showLegacyModalis declared asref(false)(:371), and no code in the repository ever sets it to true. Its only other references are the template binding and the setup return (:1367).isAdvancedMode = ref(true)(:377) is commented "Use advanced modal by default", but nothing reads it.:1373only returns it.requestManualStepConfirmation(:736-752) always opens the advanced modal (showManualStepModal).So the fallback never runs, and nothing falls back to the legacy modal if the advanced one can't be used.
Direction
The no-deletion rule says to wire the fallback in:
requestManualStepConfirmationopens the legacy modal whenisAdvancedModeis false.isAdvancedMode: a user setting, or a fallback whenAdvancedStepConfirmationModalcan't render.If the owner decides the fallback isn't wanted, retire
showLegacyModal,isAdvancedModeand the legacy modal inTerminalModalstogether.Acceptance criteria
isAdvancedModefalse opensshowLegacyModal), with a test through that path; or it's removed fromTerminalWindowandTerminalModals, together withisAdvancedMode, and the stories are updated.