fix(terminal): render TerminalModals in TerminalWindow, with each action reporting the parent's real outcome (#16285)

[mrveiss]

Closes #16285. Closes #16315 (folded in, see below). Refs #16253, #16274 and #16312 (found while fixing this).

Thinking Path

• TerminalModals.vue was an extraction of TerminalWindow's four confirmation modals, but nothing rendered it. It reported every action as a success after a fixed setTimeout (six "Simulate async operation" sites), and it raced each action against a deadline timer that it never cleared.
• TerminalWindow kept rendering its own inline copies, some with hardcoded English ("⚡ Execute Command", "❌ Cancel", "Step N of M").
• This PR finishes the extraction. TerminalWindow renders TerminalModals, and each action's outcome comes from the parent, as a function prop that TerminalModals awaits. That follows the issue's "promise-returning prop" direction.
• For that outcome to be real, the parent actions had to stop swallowing it:
  • executeConfirmedCommand and confirmWorkflowStep now await the send (executeCommand and executeAutomatedCommand return sendInput's promise).
  • confirmEmergencyKill logs the failure to the terminal, then rethrows it.
  • The modal's reconnect uses a new reconnectFromModal, which closes the modal only once connected and rejects with the connection error.
  • The header's reconnect button behaves as before: a failure there is reported in the terminal output only.
• withTimeout lands here as its first consumer (AC4). It's the utility drafted for canonical(frontend): SystemStatusDetails is declared three times, and SystemAlert twice with no importers #16253, at autobot-frontend/src/utils/withTimeout.ts. canonical(frontend): SystemStatusDetails is declared three times, and SystemAlert twice with no importers #16253 will import it rather than add it. When it rebases, it drops its own copy and its test, and a grep confirms a single definition.

What Changed

• components/terminal/TerminalWindow.vue
  • Renders <TerminalModals> instead of the inline reconnect, command, kill and legacy workflow-step modals, which are removed.
  • Drops the CSS that only those modals used. The keyframes it keeps (pulse-success, pulse, blink, flash) are each still used.
  • The action changes are the ones listed above. The legacy handlers also now close showLegacyModal, which they never did before.
• components/terminal/TerminalModals.vue
  • Adds six function props: reconnectAction, executeCommandAction, emergencyKillAction, confirmStepAction, skipStepAction, manualControlAction.
  • One runAction does withTimeout(Promise.resolve().then(action), …). It reports success only after the action settles, and shows the error otherwise.
  • The six action emits are gone (no parent bound them). The cancel and hide emits stay.
  • Success, timeout and fallback-error messages go through i18n.
• utils/withTimeout.ts, plus its test (see above).
• All 11 locales
  • 17 new terminal.modals keys, translated in every locale.
  • Drops the 15 terminal.window keys that only the removed inline modals used. A grep finds no other reference to them.
• repo_tests/i18n_untranslated_ratchet_test.py
  • The baseline drops by 13 per locale: 13 of the dropped keys were still English everywhere.
  • Counts come from the locale JSON on base vs. the branch: ar 3754→3741, de 1631→1618, es 1680→1667, fa/he/ur 3818→3805, fr 1877→1864, lv 2272→2259, pl 2238→2225, pt 2075→2062.
• TerminalModals.stories.ts: meta-level args supply actions that resolve.
• fix(terminal): TerminalModals' message auto-hide timers are never cleared, on unmount or when a newer message replaces them #16315, folded in rather than left to a separate PR (filed as a same-file review nit, specifically to avoid a re-push during the merge-train freeze): handleError/handleSuccess's auto-hide setTimeouts were never tracked, so an unmount mid-window still fired into a disposed ref, and a second message in the same slot could be blanked early by the first message's own timer. Each of the 8 message refs (error/success × connection/command/kill/workflow) now has its own tracked timer in a Map; a new message cancels its own slot's pending timer via ActionRun.slot, and onBeforeUnmount clears whatever is left.
  • Note for reviewers: fix(terminal): clear TerminalModals' auto-hide timers on unmount and on re-show (#16315) #16397 (issue-16315-terminal-modal-timers) also targets fix(terminal): TerminalModals' message auto-hide timers are never cleared, on unmount or when a newer message replaces them #16315, filed against Dev_new_gui before this PR's runAction/withTimeout rewrite of these handlers landed. Its diff and test (.aui-dialog-actions button indices, a hand-rolled Promise.race timeout, and fix(terminal): TerminalModals' action-timeout race leaves the loser's setTimeout uncleared #16395's now-superseded guard timer) no longer match this file's current shape. This PR supersedes it.
• Tests
  • components/__tests__/TerminalModals.test.ts (new):
    • a rejecting action shows its error, and no success (AC2);
    • a synchronous throw does the same;
    • success appears only after the action settles, with no timer advanced (AC3);
    • an action that never settles fails at its withTimeout deadline (AC4);
    • workflow-step failure and success.
  • components/__tests__/TerminalWindow.test.ts:
    • TerminalModals is rendered and receives all six actions (AC1);
    • the command, kill and reconnect actions reject when the send, kill or connection fails;
    • the command action resolves once the command is sent.

Verification

Acceptance criteria, read from the pushed branch:

• AC1: TerminalWindow's template renders <TerminalModals>. modal-overlay, confirmation-modal, btn-danger, risk-level and pulse-danger have 0 matches in TerminalWindow.vue.
• AC3: "Simulate async operation" has 0 matches in TerminalModals.vue.
• AC4: the only withTimeout( call is in runAction, which all six actions go through, and export function withTimeout exists once in autobot-frontend/src.
• fix(terminal): TerminalModals' message auto-hide timers are never cleared, on unmount or when a newer message replaces them #16315's ACs, read from the pushed branch (autobot-frontend/src/components/terminal/TerminalModals.vue):
  • AC "each timer tracked per slot, new message cancels it": scheduleAutoHide clears autoHideTimers.get(slotKey) before arming a new one; ActionRun.slot threads a distinct key ('connection'|'command'|'kill'|'workflow') to each of the four action families' handleError/handleSuccess calls.
  • AC "cleared in onBeforeUnmount": present, iterates autoHideTimers.values() and clears each.
  • AC "test asserts vi.getTimerCount() === 0 after unmount with a message showing": TerminalModals.test.ts::'clears every pending auto-hide timer on unmount (#16315)'.
  • AC "a later message in a slot stays visible for its full duration": TerminalModals.test.ts::"a later message in the same slot outlives the earlier message's auto-hide timer (#16315)".

CI is the evidence for vitest, vue-tsc and the ratchet. The repo's pre-commit and pre-push hooks ran on the commit.

Out of scope: #16312. The legacy workflow-step modal can never open: nothing sets showLegacyModal, and nothing reads isAdvancedMode.

Model Used

Claude Opus 5 (claude-opus-5)

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 55c61d76-9a70-4618-a003-e9147f3e7ab5

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[mrveiss]

Review at ab649870b2: approved. It merges on a settled green at that SHA after the current merge train (#16314) lands, since base stays frozen until the train's tree comparison is done. Closes #16285 is right, with #16253, #16274 and #16312 as references.

#16285 AC	Evidence
TerminalWindow renders TerminalModals; the inline copies are gone	<TerminalModals> is rendered once, and no inline modal markup is left in the template.
No success unless the parent's handler succeeded, tested with a failing handler	All six action props (reconnectAction, executeCommandAction, emergencyKillAction, and confirmStepAction / skipStepAction / manualControlAction through runWorkflowAction) go through runAction. That awaits withTimeout(Promise.resolve().then(action), …), so a rejection or a synchronous throw lands in catch and is shown. Success is reported only after the action settles. The parent makes the outcome real: reconnectFromModal connects with rethrow: true, a failed kill is rethrown (:529), and command and workflow input are awaited through sendInput. Tests: …shows the error when the parent's action rejects…, …throws synchronously, reports success only once the action settles, with no fixed delay, plus five wiring tests in TerminalWindow.test.ts (command send failure, kill failure, reconnect failure, and success after the send).
No // Simulate async operation delay remains	None remains. The two setTimeouts left only auto-hide the error message (10 s) and the success message (5 s). They're display-only and decide no outcome.
Every action deadline uses withTimeout	Yes, for all six. withTimeout races against a deadline that rejects with the given message, and clears its timer in finally. withTimeout.test.ts covers resolve, pass-through rejection and the deadline.

i18n: 18 new keys in en.json (the 17 terminal.modals leaves plus the modals object), present in all 11 locales. The untranslated ratchet drops by exactly 13 in each of the ten non-English locales. No commit is authored by a placeholder identity.

Nit, non-blocking: the two auto-hide timers aren't cleared on unmount, so closing the terminal within 5–10 s of a result sets a value on an unmounted component. Clearing them in onUnmounted would tidy that up.

[mrveiss]

Correction to my review above: the branch adds 17 i18n keys, not 18. Comparing leaf paths between base and branch en.json: 17 added, all under terminal.modals (which already existed on base with 36 keys), and 15 removed, all under terminal.window. My "plus the modals object" was a guessed explanation for a count my line grep inflated. That object is not new. All 17 added keys are present in all 11 locales, so that check is unchanged, and the approval stands. The auto-hide-timer nit is filed as #16315.

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No new hardcoded values of either class — ssot and other both block.

Known backlog in pipeline-scripts/hardcoded_values_baseline.txt is suppressed and tracked in #14371.

[mrveiss]

Root-caused the shard 4/12 red: this PR's own consolidation work lowered four measured counts below the pinned ratchet baseline in frontend_fragmentation_ratchet_test.py, so the guard read the improvement as a regression. Lowered the four literals to match: modal 26→25, button_definition_files 102→101, css_rule_declarations 9434→9405, distinct_class_names 5627→5623.

Also folded in #16315 (same-file review nit, filed to avoid a re-push during the merge-train freeze): TerminalModals.vue's handleError/handleSuccess auto-hide timers were never tracked or cleared, so an unmounted component still fired timers into torn-down refs, and a second message in the same slot could be blanked early by the first message's own timer. Each message slot's timer is now tracked in a Map, a new message cancels its slot's pending timer, and onBeforeUnmount clears whatever is left. Added the two tests #16315's acceptance criteria asked for (fake-timer unmount asserting vi.getTimerCount() === 0, and a same-slot two-message test asserting the later message survives its full window).

Pushed as 54eb554.

[mrveiss]

Content review: approve.

Real correctness fix, not just a refactor: the old TerminalModals.vue reported every action as a fake success after a fixed setTimeout regardless of what the actual backend action did (the "Simulate async operation" pattern) — runAction now genuinely awaits the parent's action and only reports success once it settles, propagating real rejections through handleError. Verified withTimeout.ts clears its deadline timer via .finally() regardless of which side of the race wins — fixes the described leak in the bare Promise.race pattern.

The folded-in #16315 fix is sound: per-slot (Map<string, Timer>) auto-hide tracking replaces bare untracked setTimeouts, so a new message in a slot cancels that slot's own pending timer (no early-blank-by-an-earlier-message-in-the-same-slot) and onBeforeUnmount clears everything outstanding. Confirmed #16397 (the other PR that targeted #16315 against a now-stale diff shape) is already closed — no dangling duplicate to reconcile.

Fragmentation ratchet moves are all in the shrinking direction (distinct_class_names 5627→5623, css_rule_declarations 9434→9405, modal 26→25, button_definition_files 102→101) — consistent with deleting the duplicated inline modal markup TerminalWindow used to carry alongside the unused TerminalModals component.

closingIssuesReferences=[16285,16315], matches its own two Closes lines exactly. CI green, no missing required contexts. Ready.