Repository navigation
fix(terminal): one automation-pacing composable that clears its timers, and i18n for the terminal output lines (#16396, #17942, #17946) - #17950
Merged
Conversation
…s on unmount (#16396, #17942) TerminalWindow and WorkflowAutomation each carried a copy of the step pacing (1000/2000/1500 ms setTimeout chains) and neither cleared it on unmount. useAutomationPacing now owns the queue/step pacing, tracks every timer and clears them in onScopeDispose; TerminalWindow's focus-retry timers ride the same tracker. The workflow-start messages and the default step description move to terminal.automation.* in all 11 locales. TerminalWindow.vue 1787 -> 1755 lines, WorkflowAutomation.vue 422 -> 358.
…the cursor-blink interval (#16396, #17942) Review of the composable commit: #16396 asks for a fake-timers test per component, and the composable test only used a bare effectScope. The new file mounts TerminalWindow and WorkflowAutomation and proves no pacing callback fires after unmount. TerminalWindow's cursor-blink setInterval was never stored or cleared; it is now cleared in onUnmounted.
…demo workflow once (#17946, #17942) The 29 remaining hard-coded English output lines in TerminalWindow and WorkflowAutomation move to terminal.automation.* and terminal.events.* in all 11 locales. The demo workflow, duplicated verbatim in both components, becomes exampleWorkflow(t) in useAutomationPacing. TerminalWindow.vue 1758 -> 1726 lines, WorkflowAutomation.vue 358 -> 327.
Contributor
|
Warning Review limit reachedYou'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 55 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (16)
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. Comment |
Contributor
✅ SSOT Configuration Compliance: Passing🎉 No new hardcoded values of either class — Known backlog in |
This was referenced Oct 4, 2026
mrveiss
added a commit
that referenced
this pull request
Oct 4, 2026
…d the stream socket carries its credential (#13102, #17004) * fix(voice): shape the spoken copy of replies for speech, and authenticate the TTS stream socket (#13102, #17004) Replies were read aloud as symbol soup: markdown, URLs, file paths and fenced code. createSpeechShaper (utils/ttsSentences) shapes only the spoken copy -- links to their text, inline code to its contents, URLs and paths to a localized placeholder, fenced blocks dropped with fence state and a split ``` marker carried across streamed slices. ChatInterface's streaming path and useVoiceConversation's reply path both use it; the transcript and the streaming cursor still run on the raw text. The TTS stream WebSocket opened with no credential, so the authenticated /api/voice/stream route closed every connection with 4001. It now sends the bearer subprotocol like the other migrated clients. * test(voice): the pre-roll suite mocks the TTS socket's auth helper (#17004) The bearer subprotocol comes from buildAuthenticatedWsSubprotocols, which reads the user store. The pre-roll suite has no active Pinia, so the call threw inside _connectTtsWs before the WebSocket was constructed and all 18 cases timed out waiting for the socket. Production always has Pinia. * i18n(terminal): translate the three terminal strings es/pt had as English (#17946) #17950 shipped terminal.automation.manualCommand (es, pt) and terminal.events.error (es) with values identical to English, so the untranslated-strings ratchet failed: es 1665 > 1663, pt 2059 > 2058. * fix(voice): fences open and close only at line start by CommonMark's rules, the shaper sees exact spans, and priming reads skipped text (#13102, #17953) Review on #17954: - split('```') treated an inline ``` as a fence and dropped the prose after it, and let a shorter ``` line close a four-backtick block. Fences now open and close only at line start; a close needs the same character and at least the opening length. ~~~ fences are handled too (#17953, part). - extractCompleteSentences also returns exact spans with their whole trailing whitespace, so a newline before a fence is never lost between slices; the stream-end remainder keeps its leading newline. - _primeTtsCursor feeds the skipped text to a fresh shaper unspoken, so a fence already open when voice is enabled stays closed to TTS. - Windows drive paths (C:\\Users\\...) are spoken as the path placeholder.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thinking Path
#16396 reported uncleared automation-pacing
setTimeoutchains in two components. Those chains turned out to be one piece of logic implemented twice, inline inTerminalWindow.vueand inWorkflowAutomation.vue. Fixing the leak in both copies would have grown two oversized files and kept the duplicate. So the pacing moves into one composable that owns its timers and clears them on scope dispose (#17942), and the leak is fixed once.Both files also wrote every terminal output line, and the duplicated demo workflow, in hard-coded English (#17946). Issues touching the same files go in one PR, so the i18n work rides here too.
#16395, filed alongside #16396, was already delivered by #16313's
withTimeoutand was closed with evidence; this PR does not touch it.What Changed
composables/useAutomationPacing.ts(new).useAutomationPacingprovidesprocessNextAutomationStep,scheduleNextAutomationStep,startFirstAutomationStepandscheduleTracked. Every timer it starts is tracked and cleared inonScopeDispose. It also exposes:buildAutomationSteps: the queue shape, previously built twiceworkflowStartedLinesexampleWorkflow(t): the demo workflow, previously duplicated verbatimTerminalWindow.vue:scheduleTracked, so they are cleared on unmount toosetIntervalwas never stored; it is now cleared inonUnmounted(found in review)WorkflowAutomation.vue: uses the composable; 422 → 327 lines. Its props, emits and exposed methods are unchanged.terminal.automation.*andterminal.events.*in all 11 locales, 31 keys with matching{command}/{name}/{count}/{step}/{total}/{description}/{message}/{error}placeholders. The onlycontent:literal left in either file is the${currentPrompt}${command}echo, which is not UI text.composables/__tests__/useAutomationPacing.test.ts: pacing order and delays, the paused no-op, and no callback after scope dispose (including a step offer already dequeued), plus the builders.components/__tests__/TerminalAutomationUnmount.test.ts: one mount/unmount fake-timers case per component, as fix(terminal): automation-pacing setTimeout chains in TerminalWindow/WorkflowAutomation are never cleared on unmount #16396 asks.TerminalWindow.test.tsandTerminalModals.vueare untouched, which keeps them clear of the upcoming frontend file-size ratchet's pins.Merge order: the frontend file-size ratchet branch is held until this lands, coordinated with the session that owns it.
Follow-up filed, not in scope: #17943. TerminalWindow is half-decomposed, and four extracted components (WorkflowAutomation, TerminalInput, TerminalOutput, TerminalStatusBar) are mounted nowhere. It is
blocked_by#17942.Closes #16396
Closes #17942
Closes #17946
Verification
Local, in
autobot-frontend/:npx oxlint <changed files>: rc=0npx eslint <changed files>: rc=0npx vue-tsc --noEmit -p tsconfig.app.json: rc=0, 0error TScheck json, thet(plural-key, {count})check, hardcoded values, andCanonical check — FrontendThe test suites run in CI; the repo rule is not to run codebase code locally.
A
code-reviewerpass on the first commit found no blocking defects. Its two low findings were the missing per-component unmount test and the cursor-blink interval; both are fixed inf49b346208.Model Used
Claude Opus 5.5 (
claude-opus-5-5); review subagent on Sonnet.🤖 Generated with Claude Code