Skip to content

Move CLI autocomplete implementation to use Vue / Pinia#4854

Open
haslinghuis wants to merge 5 commits intobetaflight:masterfrom
haslinghuis:fix-cli-tab-completion
Open

Move CLI autocomplete implementation to use Vue / Pinia#4854
haslinghuis wants to merge 5 commits intobetaflight:masterfrom
haslinghuis:fix-cli-tab-completion

Conversation

@haslinghuis
Copy link
Member

@haslinghuis haslinghuis commented Feb 7, 2026

  • currently autocomplete does not work

Summary by CodeRabbit

Release Notes

  • New Features

    • Added CLI autocomplete dropdown with real-time suggestions, highlighted matches, and full keyboard navigation (arrow keys, Tab, Enter) and mouse selection support.
  • Bug Fixes

    • Improved Tab and Enter key handling to prevent conflicts with autocomplete behavior.
  • Tests

    • Added comprehensive test suite for autocomplete strategies and edge cases.

@haslinghuis haslinghuis added this to the 2026.6 milestone Feb 7, 2026
@haslinghuis haslinghuis self-assigned this Feb 7, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 7, 2026

Walkthrough

This PR introduces a new Vue 3 CLI autocomplete component with dropdown suggestions, integrates it into the CliTab component, adds a Pinia store for state management, enhances event handling in the composable to support synthetic events, improves the legacy CliAutoComplete.js with single-match fallback behavior, and provides comprehensive test coverage.

Changes

Cohort / File(s) Summary
New Vue Autocomplete Component
src/components/cli/CliAutocomplete.vue
New single-file Vue 3 component providing in-page CLI autocompletion with dropdown suggestions, strategy-based text replacement, keyboard navigation (ArrowUp/Down, Enter, Tab), and integration with Pinia stores for cached suggestion data and FC config.
Vue Integration
src/components/tabs/CliTab.vue
Integrated CliAutocomplete component into the CLI tab, wired autocomplete events (@apply, @send) to handlers, added Pinia store integration for autocomplete state synchronization on build lifecycle, and implemented handlers for replacement and send operations.
State Management
src/stores/cliAutocomplete.js
New Pinia store managing autocomplete state including cache, builder configuration, and forceOpen flag; provides mutation helpers (setCache, setBuilder) and synchronization methods (syncFromCli, openLater) to interface with legacy global CliAutoComplete.
Event Handling Enhancement
src/composables/useCli.js
Enhanced to distinguish synthetic (isTrusted === false) Tab/Enter events from real user events, prevent interference with autocomplete selection, defer to autocomplete for Enter key handling when open, and synchronize live DOM input values to avoid race conditions.
Legacy Autocomplete Enhancement
src/js/CliAutoComplete.js
Enhanced with single-match fallback behavior triggering synthetic Tab events, expanded forceOpen handling across multiple strategies, improved minChars settings for broader early suggestions, added try/catch error handling for robustness, and refined search callbacks to implement fallback replacements.
Test Suite
test/CliAutoComplete.test.js
New comprehensive test suite with mock textarea implementation, tests for strategy registration and command/resource suggestions, single-match fallback behavior, resource index placeholder generation, feature strategy behavior, and Pinia store synchronization.

Sequence Diagram

sequenceDiagram
    actor User
    participant CliTab as CliTab Component
    participant CliAutocomplete as CliAutocomplete Component
    participant Pinia as Pinia Store
    participant CliAutoComplete as Legacy CliAutoComplete
    participant Textarea as Textarea

    User->>Textarea: Types in CLI input
    Textarea->>CliAutocomplete: keydown/input events
    CliAutocomplete->>Pinia: Fetch cache & FC config
    Pinia-->>CliAutocomplete: Return cached suggestions
    CliAutocomplete->>CliAutoComplete: Trigger search via strategies
    CliAutoComplete-->>CliAutocomplete: Return matching suggestions
    CliAutocomplete->>CliAutocomplete: Render dropdown, highlight matches
    User->>CliAutocomplete: Select suggestion (keyboard/mouse)
    CliAutocomplete->>CliAutocomplete: Apply replacement logic
    CliAutocomplete->>Textarea: Update text content
    Textarea->>CliTab: Emit input/apply event
    CliTab->>CliTab: onAutocompleteApply handler
    User->>CliAutocomplete: Press Enter/Tab to send
    CliAutocomplete->>CliTab: Emit send event
    CliTab->>CliTab: onAutocompleteSend handler
    CliTab->>CliTab: Execute command from textarea
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • UI: Migrating Cli to VUE (step 1) #4824: Directly related—both PRs modify the same CLI/autocomplete codepaths including useCli.js, CliAutoComplete.js behavior, and CliTab component integration for Vue migration and autocomplete functionality.

Suggested reviewers

  • sugaarK
  • nerdCopter
🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description is empty, failing to provide context, implementation details, or justification for the significant architectural changes introduced in this refactor. Add a detailed description explaining the rationale, implementation approach, testing considerations, and any breaking changes or migration notes for this Vue/Pinia refactor.
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: refactoring CLI autocomplete from vanilla JS to use Vue and Pinia state management across multiple files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 7, 2026

Quality Gate Failed Quality Gate failed

Failed conditions
2 Security Hotspots
6.0% Duplication on New Code (required ≤ 3%)
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@github-actions
Copy link
Contributor

github-actions bot commented Feb 7, 2026

🎉 Do you want to test this code? 🎉

⚠️ CAUTION: The build may be unstable and result in corrupted configurations or data loss. Use only for testing! ⚠️

@haslinghuis haslinghuis moved this to App in 2026.6.0 Feb 7, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/js/CliAutoComplete.js (1)

285-292: ⚠️ Potential issue | 🟡 Minor

Potential TypeError when val is shorter than text.

Line 287 evaluates val[text.length].match(/\s/) without guarding against val[text.length] being undefined. If the textarea value is ever shorter than the text parameter passed by the textcomplete library (e.g., during rapid edits or race conditions with the fallback replacement logic), this will throw. The short-circuit only protects the val.length === text.length case.

🛡️ Suggested guard
 const contexter = function (text) {
     const val = $textarea.val();
-    const result = val.length === text.length || val[text.length].match(/\s/);
+    const result = val.length === text.length || (val.length > text.length && /\s/.test(val[text.length]));
     if (result) {
         return true;
     }
     return false;
 };
🤖 Fix all issues with AI agents
In `@src/components/cli/CliAutocomplete.vue`:
- Around line 366-373: The returned destructured primitives break reactivity: do
not return snapshot values for state.visible, state.selectedIndex, and
state.term from the setup function; instead preserve reactivity by using
toRefs(state) (so the template binds to reactive refs for
visible/selectedIndex/term) or simply return the reactive state object itself
and update template usage accordingly; update the return that currently lists
suggestions, visible, selectedIndex, term, highlight, applySuggestion to return
suggestions and the reactive refs (via toRefs(state)) or state plus highlight
and applySuggestion.
- Around line 299-326: The onKeyDown listener in CliAutocomplete.vue (function
onKeyDown) conflicts with the legacy handler bound in CliTab.vue
(cli.handleCommandKeyDown) when the legacy textcomplete autocomplete is enabled;
update the code so only one system responds by either (A) adding an early guard
in onKeyDown to return immediately if the legacy system is active (use
CliAutoComplete.isEnabled() or similar) so Tab/Enter are ignored by
tryOpen/applySuggestion when legacy is enabled, or (B) make the attachment of
this listener conditional so it is not added when CliAutoComplete.isEnabled() is
true; ensure you reference tryOpen, applySuggestion, state.visible and the
legacy methods CliAutoComplete.openLater()/isOpen() when implementing the guard
so only one autocomplete system processes Tab/Enter.

In `@src/components/tabs/CliTab.vue`:
- Around line 218-222: The onAutocompleteSend handler references
cli.executeCommands which is not returned from the useCli composable, so the
call silently does nothing; update the useCli composable to include
executeCommands in its returned object (export the existing executeCommands
function under that name) and then update onAutocompleteSend (in the
onAutocompleteSend function) to call cli.executeCommands(outString) directly (or
guard with a clear conditional) instead of relying on the && short-circuit so
the emitted "send" payload is actually handled.

In `@src/stores/cliAutocomplete.js`:
- Around line 42-53: The openLater function sets forceOpen.value = !!force but
never resets it to false, causing store.forceOpen to remain true after the first
forced open; update openLater (and related logic around forceOpen) to reset
forceOpen.value back to false after the delegated call completes or mirror the
original CliAutoComplete.openLater pattern by scheduling a synchronous clear
(e.g., via setTimeout or by hooking the delegate's completion) so that
forceOpen.value is true only for the intended short window; change the behavior
in the openLater function (referencing openLater, forceOpen, and
window.CliAutoComplete.openLater) to ensure forceOpen.value is reset to false
after the temporary forced-open period.

In `@test/CliAutoComplete.test.js`:
- Around line 70-82: The tests mutate FC.CONFIG.flightControllerVersion and the
shared beforeEach only resets CliAutoComplete.cache and builder, causing state
leakage; update the test setup to save the original FC.CONFIG (or at minimum
FC.CONFIG.flightControllerVersion) before tests run and restore it in the shared
beforeEach or an afterEach so each test (including "resource strategy" and
"resource fallback") gets a fresh config; locate FC.CONFIG references in the
test file and add code to capture the original value and reassign it back after
each test.
🧹 Nitpick comments (4)
src/js/CliAutoComplete.js (2)

406-457: Duplicated single-match fallback logic between command and resource strategies.

The fallback block here (lines 416-448) is nearly identical to the one in the resource strategy (lines 566-606). Both perform the same sequence: check forceOpen && res.length === 1, compute replacement via strategyThis.replace, substitute $n placeholders, set the textarea value, move cursor, trigger "input", set _fallbackApplied, and conditionally fire sendOnEnter. Consider extracting a shared helper like applySingleMatchFallback(self, $textarea, strategyThis, res, basicReplacer, sendOnEnter) to eliminate the duplication.

♻️ Sketch of extracted helper
+// Add near the top of _initTextcomplete, after helper definitions
+function applySingleMatchFallback(strategyCtx, res) {
+    if (!self.forceOpen || res.length !== 1) return;
+    try {
+        const value = res[0];
+        const replacementTemplate = strategyCtx.replace
+            ? strategyCtx.replace.call(strategyCtx, value)
+            : basicReplacer(value);
+        if (typeof replacementTemplate !== "string") return;
+
+        const currentVal = $textarea.val();
+        const m = (strategyCtx.match && strategyCtx.match.exec(currentVal)) || [];
+        const replacement = replacementTemplate.replace(/\$(\d+)/g, (_, n) => m[parseInt(n, 10)] || "");
+
+        $textarea.val(replacement);
+        try { $textarea[0].selectionStart = $textarea[0].selectionEnd = replacement.length; } catch (e) {}
+        try { $textarea.trigger("input"); } catch (e) {}
+        self._fallbackApplied = true;
+        if (sendOnEnter) {
+            $textarea.trigger($.Event("keypress", { which: 13 }));
+        }
+    } catch (error) {
+        // fallback error suppressed
+    }
+}

Then in both strategies replace the inline block with:

 searcher(
     term,
     function (res) {
         callback(res);
-        try {
-            if (self.forceOpen && res.length === 1) {
-                // ... 30+ lines of duplicated code ...
-            }
-        } catch (error) {
-            // fallback error suppressed
-        }
+        applySingleMatchFallback(strategyThis, res);
     },
     ...
 );

314-318: Leftover debug scaffolding — empty conditional body.

Lines 316-318 contain an if block that checks for Tab/Enter key events but does nothing. This appears to be a debug trace that was partially removed. Either add meaningful logging or remove the dead block.

🧹 Remove dead code
 const handleCommandKeyDown = (event) => {
-    // Debug: log key events relevant to autocomplete
-    if (event.which === tabKeyCode || event.which === enterKeyCode) {
-        // key event (tab/enter) occurred
-    }
-
     // Ignore synthetic Tab events triggered by textcomplete which use jQuery.trigger
src/composables/useCli.js (1)

314-325: Empty debug conditional and clear synthetic-Tab guard.

Lines 316-318 are a no-op if block (same issue as noted elsewhere — leftover debug scaffold). The synthetic Tab guard on lines 323-325 is correct: jQuery-triggered events have isTrusted === false, and letting them pass through prevents interfering with textcomplete's single-candidate selection.

Consider removing the empty block:

🧹 Remove empty block
 const handleCommandKeyDown = (event) => {
-    // Debug: log key events relevant to autocomplete
-    if (event.which === tabKeyCode || event.which === enterKeyCode) {
-        // key event (tab/enter) occurred
-    }
-
     // Ignore synthetic Tab events triggered by textcomplete ...
src/components/cli/CliAutocomplete.vue (1)

321-325: Unused variable val.

val on line 323 is assigned but never read — applySuggestion(state.selectedIndex) on line 324 re-fetches the value internally.

🧹 Remove unused variable
         } else if (e.key === "Enter") {
             e.preventDefault();
-            const val = state.suggestions[state.selectedIndex];
             applySuggestion(state.selectedIndex);
         }

Comment on lines +299 to +326
function onKeyDown(e) {
const el = props.textareaRef.value;
if (!el) return;

if (e.key === "Tab") {
e.preventDefault();
// force open
tryOpen(true);
return;
}

if (!state.visible) {
// nothing to do special
return;
}

if (e.key === "ArrowDown") {
e.preventDefault();
state.selectedIndex = (state.selectedIndex + 1) % state.suggestions.length;
} else if (e.key === "ArrowUp") {
e.preventDefault();
state.selectedIndex = (state.selectedIndex - 1 + state.suggestions.length) % state.suggestions.length;
} else if (e.key === "Enter") {
e.preventDefault();
const val = state.suggestions[state.selectedIndex];
applySuggestion(state.selectedIndex);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's find the exact structure of CliAutocomplete.vue and check line 337
wc -l src/components/cli/CliAutocomplete.vue

Repository: betaflight/betaflight-configurator

Length of output: 120


🏁 Script executed:

# Check the listener attachment around line 337 in CliAutocomplete.vue
sed -n '330,345p' src/components/cli/CliAutocomplete.vue

Repository: betaflight/betaflight-configurator

Length of output: 610


🏁 Script executed:

# Find CliTab.vue and examine its structure
find . -name "CliTab.vue" -type f

Repository: betaflight/betaflight-configurator

Length of output: 110


🏁 Script executed:

# Check for useCli.js and examine handleCommandKeyDown
find . -name "useCli.js" -type f

Repository: betaflight/betaflight-configurator

Length of output: 105


🏁 Script executed:

# Search for references to both autocomplete systems and any guards
rg -n "CliAutocomplete|cliAutocomplete|handleCommandKeyDown|openLater" --type=vue --type=js -g '!node_modules' -g '!test/'

Repository: betaflight/betaflight-configurator

Length of output: 105


🏁 Script executed:

# Check CliTab.vue structure to see if `@keydown` binding exists
cat -n src/components/tabs/CliTab.vue | head -80

Repository: betaflight/betaflight-configurator

Length of output: 4139


🏁 Script executed:

# Search for the textarea element and keydown binding in CliTab.vue
rg -n "@keydown|handleCommandKeyDown|CliAutocomplete" -A3 -B3 src/components/tabs/CliTab.vue

Repository: betaflight/betaflight-configurator

Length of output: 1414


🏁 Script executed:

# Examine useCli.js for handleCommandKeyDown function
cat -n src/composables/useCli.js | head -100

Repository: betaflight/betaflight-configurator

Length of output: 3920


🏁 Script executed:

# Search for handleCommandKeyDown in useCli.js
rg -n "handleCommandKeyDown" -A10 src/composables/useCli.js

Repository: betaflight/betaflight-configurator

Length of output: 1046


🏁 Script executed:

# Search for any feature flags or guards that might control which autocomplete is active
rg -n "openLater|isEnabled|isMigration|CliAutocomplete" -g '!node_modules' -g '!test/' .

Repository: betaflight/betaflight-configurator

Length of output: 6389


🏁 Script executed:

# Get the full handleCommandKeyDown function
sed -n '314,370p' src/composables/useCli.js

Repository: betaflight/betaflight-configurator

Length of output: 2434


🏁 Script executed:

# Check CliAutoComplete.isEnabled() implementation
rg -n "CliAutoComplete.isEnabled\s*=" -A8 src/js/CliAutoComplete.js

Repository: betaflight/betaflight-configurator

Length of output: 464


🏁 Script executed:

# Check if there's any logic in CliAutocomplete.vue that disables the legacy system
rg -n "isEnabled|CliAutoComplete" -B3 -A3 src/components/cli/CliAutocomplete.vue | head -50

Repository: betaflight/betaflight-configurator

Length of output: 60


🏁 Script executed:

# Check the full context around line 331 in useCli.js where isEnabled is checked
sed -n '320,360p' src/composables/useCli.js

Repository: betaflight/betaflight-configurator

Length of output: 1878


🏁 Script executed:

# Verify the exact flow: check if Vue's `@keydown` directive runs after addEventListener
# Let's also check when attachListeners is called in CliAutocomplete.vue
sed -n '350,380p' src/components/cli/CliAutocomplete.vue

Repository: betaflight/betaflight-configurator

Length of output: 872


🏁 Script executed:

# Check the full onKeyDown function in CliAutocomplete.vue to verify state.visible guard
sed -n '299,330p' src/components/cli/CliAutocomplete.vue

Repository: betaflight/betaflight-configurator

Length of output: 1136


🏁 Script executed:

# Verify when textcomplete is initialized/when CliAutoComplete.isEnabled() becomes true
rg -n "configEnabled|builder.state" -B2 -A2 src/js/CliAutoComplete.js | head -40

Repository: betaflight/betaflight-configurator

Length of output: 1363


🏁 Script executed:

# Check if CliAutocomplete.vue has any logic to disable/gate the legacy system
rg -n "setEnabled|configEnabled|CliAutoComplete\." src/components/cli/CliAutocomplete.vue

Repository: betaflight/betaflight-configurator

Length of output: 60


🏁 Script executed:

# Check CliTab.vue to see if it manages the legacy system state based on Vue component
rg -n "setEnabled|CliAutoComplete\." src/components/tabs/CliTab.vue

Repository: betaflight/betaflight-configurator

Length of output: 60


🏁 Script executed:

# Verify what tryOpen does vs openLater 
rg -n "function tryOpen|const tryOpen" -A8 src/components/cli/CliAutocomplete.vue

Repository: betaflight/betaflight-configurator

Length of output: 429


🏁 Script executed:

# Check what tryOpen actually does - find the full implementation
sed -n '202,230p' src/components/cli/CliAutocomplete.vue

Repository: betaflight/betaflight-configurator

Length of output: 1458


🏁 Script executed:

# Check openLater implementation in the legacy CliAutoComplete.js
rg -n "CliAutoComplete.openLater\s*=" -A12 src/js/CliAutoComplete.js

Repository: betaflight/betaflight-configurator

Length of output: 475


🏁 Script executed:

# Verify the event attachment order - are both listeners attached to the same element?
rg -n "addEventListener.*keydown|@keydown" src/components/cli/CliAutocomplete.vue src/components/tabs/CliTab.vue

Repository: betaflight/betaflight-configurator

Length of output: 261


Dual keydown listeners on the same textarea will conflict when the legacy autocomplete system is enabled.

This component attaches its own keydown listener (CliAutocomplete.vue line 337) to the textarea element, while CliTab.vue also binds @keydown="cli.handleCommandKeyDown" on the same element. Both fire for Tab and Enter:

  • Tab: Both tryOpen(true) (Vue system) and CliAutoComplete.openLater(true) (legacy textcomplete) are called, activating competing autocomplete systems simultaneously.
  • Enter: Both handlers call preventDefault() and process the command, with synchronization relying on checking separate state systems (state.visible vs CliAutoComplete.isOpen()).

The guard via CliAutoComplete.isEnabled() determines which system is primary but doesn't prevent both from activating when the legacy system is enabled. This double-processing can cause competing dropdowns and unclear command handling behavior. Verify whether this is intentional during a migration period, or if one listener should be disabled when the other is active.

🤖 Prompt for AI Agents
In `@src/components/cli/CliAutocomplete.vue` around lines 299 - 326, The onKeyDown
listener in CliAutocomplete.vue (function onKeyDown) conflicts with the legacy
handler bound in CliTab.vue (cli.handleCommandKeyDown) when the legacy
textcomplete autocomplete is enabled; update the code so only one system
responds by either (A) adding an early guard in onKeyDown to return immediately
if the legacy system is active (use CliAutoComplete.isEnabled() or similar) so
Tab/Enter are ignored by tryOpen/applySuggestion when legacy is enabled, or (B)
make the attachment of this listener conditional so it is not added when
CliAutoComplete.isEnabled() is true; ensure you reference tryOpen,
applySuggestion, state.visible and the legacy methods
CliAutoComplete.openLater()/isOpen() when implementing the guard so only one
autocomplete system processes Tab/Enter.

Comment on lines +366 to +373
return {
suggestions: state.suggestions,
visible: state.visible,
selectedIndex: state.selectedIndex,
term: state.term,
highlight,
applySuggestion,
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Reactivity is lost — visible, selectedIndex, and term are returned as snapshot primitives.

Destructuring primitive values from a reactive() object in the return severs their reactive connection. The template binds to visible, selectedIndex, and term, but these are frozen at their initial values (false, 0, ""). Critically, v-if="visible" will always be false, so the dropdown will never render.

Use toRefs to preserve reactivity, or return the reactive object itself:

🐛 Fix: use toRefs
-import { defineComponent, reactive, onBeforeUnmount, nextTick, watch } from "vue";
+import { defineComponent, reactive, onBeforeUnmount, nextTick, watch, toRefs } from "vue";
         return {
-            suggestions: state.suggestions,
-            visible: state.visible,
-            selectedIndex: state.selectedIndex,
-            term: state.term,
+            ...toRefs(state),
             highlight,
             applySuggestion,
         };

Alternatively, return state directly and use state.visible, state.suggestions, etc. in the template.

🤖 Prompt for AI Agents
In `@src/components/cli/CliAutocomplete.vue` around lines 366 - 373, The returned
destructured primitives break reactivity: do not return snapshot values for
state.visible, state.selectedIndex, and state.term from the setup function;
instead preserve reactivity by using toRefs(state) (so the template binds to
reactive refs for visible/selectedIndex/term) or simply return the reactive
state object itself and update template usage accordingly; update the return
that currently lists suggestions, visible, selectedIndex, term, highlight,
applySuggestion to return suggestions and the reactive refs (via toRefs(state))
or state plus highlight and applySuggestion.

Comment on lines +218 to +222
function onAutocompleteSend() {
const outString = cli.commandInputRef.value ? cli.commandInputRef.value.value : cli.state.commandInput;
cli.executeCommands && cli.executeCommands(outString);
cli.state.commandInput = "";
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

cli.executeCommands is not exposed by useCli() — this handler silently does nothing.

useCli() does not include executeCommands in its return object (see src/composables/useCli.js lines 587-612). So cli.executeCommands is always undefined, and the && short-circuit on line 220 causes the command to be silently dropped when the CliAutocomplete component emits "send".

Proposed fix: expose `executeCommands` from `useCli`

In src/composables/useCli.js, add executeCommands to the return object:

     return {
         state,
         history,
         windowWrapperRef,
         cliWindowRef,
         commandInputRef,
         snippetPreviewDialogRef,
         supportWarningDialogRef,
         initialize,
         cleanup,
         clearHistory,
         saveFile,
         loadFile,
         copyToClipboard,
         submitSupportRequest,
         handleCommandKeyDown,
         handleCommandKeyPress,
         handleCommandKeyUp,
         handleSupportDialogSubmit,
         handleSupportDialogCancel,
         showSupportWarningDialog,
         read,
         sendLine,
+        executeCommands,
         isSupportRequestAvailable,
         adaptPhones,
     };

Then in CliTab.vue, call it directly:

 function onAutocompleteSend() {
     const outString = cli.commandInputRef.value ? cli.commandInputRef.value.value : cli.state.commandInput;
-    cli.executeCommands && cli.executeCommands(outString);
+    cli.executeCommands(outString);
     cli.state.commandInput = "";
 }
🤖 Prompt for AI Agents
In `@src/components/tabs/CliTab.vue` around lines 218 - 222, The
onAutocompleteSend handler references cli.executeCommands which is not returned
from the useCli composable, so the call silently does nothing; update the useCli
composable to include executeCommands in its returned object (export the
existing executeCommands function under that name) and then update
onAutocompleteSend (in the onAutocompleteSend function) to call
cli.executeCommands(outString) directly (or guard with a clear conditional)
instead of relying on the && short-circuit so the emitted "send" payload is
actually handled.

Comment on lines +42 to +53
function openLater(force = false) {
try {
if (
typeof window !== "undefined" &&
window.CliAutoComplete &&
typeof window.CliAutoComplete.openLater === "function"
) {
window.CliAutoComplete.openLater(force);
forceOpen.value = !!force;
}
} catch (e) {}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

forceOpen is set but never reset to false.

openLater sets forceOpen.value = !!force only when window.CliAutoComplete exists and force is truthy. Once set to true, nothing in this store resets it to false. In the legacy CliAutoComplete.openLater, the reset happens synchronously after the setTimeout fires (line 46 of CliAutoComplete.js). If any consumer reads store.forceOpen, it will be permanently stale after the first forced open.

Either reset it after the delegate call or mirror the original pattern:

Proposed fix
 function openLater(force = false) {
     try {
         if (
             typeof window !== "undefined" &&
             window.CliAutoComplete &&
             typeof window.CliAutoComplete.openLater === "function"
         ) {
             window.CliAutoComplete.openLater(force);
             forceOpen.value = !!force;
+            // Mirror the legacy reset: forceOpen is only true during the async trigger
+            setTimeout(() => {
+                forceOpen.value = false;
+            }, 0);
         }
     } catch (e) {}
 }
🤖 Prompt for AI Agents
In `@src/stores/cliAutocomplete.js` around lines 42 - 53, The openLater function
sets forceOpen.value = !!force but never resets it to false, causing
store.forceOpen to remain true after the first forced open; update openLater
(and related logic around forceOpen) to reset forceOpen.value back to false
after the delegated call completes or mirror the original
CliAutoComplete.openLater pattern by scheduling a synchronous clear (e.g., via
setTimeout or by hooking the delegate's completion) so that forceOpen.value is
true only for the intended short window; change the behavior in the openLater
function (referencing openLater, forceOpen, and
window.CliAutoComplete.openLater) to ensure forceOpen.value is reset to false
after the temporary forced-open period.

Comment on lines +70 to +82
beforeEach(() => {
CliAutoComplete.cache = {
commands: [],
resources: [],
resourcesCount: {},
settings: [],
settingsAcceptedValues: {},
feature: [],
beeper: ["ALL"],
mixers: [],
};
CliAutoComplete.builder = { state: "reset", numFails: 0 };
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

FC.CONFIG mutations leak between tests.

The beforeEach resets CliAutoComplete.cache and builder but not FC.CONFIG.flightControllerVersion, which is mutated in the "resource strategy" and "resource fallback" tests (lines 112, 139). Since vitest runs tests sequentially within a file, later tests may see a stale flightControllerVersion value. Consider saving and restoring FC.CONFIG in the shared beforeEach, or adding per-test cleanup.

🧪 Suggested fix
 beforeEach(() => {
     CliAutoComplete.cache = {
         commands: [],
         resources: [],
         resourcesCount: {},
         settings: [],
         settingsAcceptedValues: {},
         feature: [],
         beeper: ["ALL"],
         mixers: [],
     };
     CliAutoComplete.builder = { state: "reset", numFails: 0 };
+    // Reset FC config to avoid leaking between tests
+    FC.CONFIG = FC.CONFIG || {};
+    FC.CONFIG.flightControllerVersion = "0.0.0";
 });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
beforeEach(() => {
CliAutoComplete.cache = {
commands: [],
resources: [],
resourcesCount: {},
settings: [],
settingsAcceptedValues: {},
feature: [],
beeper: ["ALL"],
mixers: [],
};
CliAutoComplete.builder = { state: "reset", numFails: 0 };
});
beforeEach(() => {
CliAutoComplete.cache = {
commands: [],
resources: [],
resourcesCount: {},
settings: [],
settingsAcceptedValues: {},
feature: [],
beeper: ["ALL"],
mixers: [],
};
CliAutoComplete.builder = { state: "reset", numFails: 0 };
// Reset FC config to avoid leaking between tests
FC.CONFIG = FC.CONFIG || {};
FC.CONFIG.flightControllerVersion = "0.0.0";
});
🤖 Prompt for AI Agents
In `@test/CliAutoComplete.test.js` around lines 70 - 82, The tests mutate
FC.CONFIG.flightControllerVersion and the shared beforeEach only resets
CliAutoComplete.cache and builder, causing state leakage; update the test setup
to save the original FC.CONFIG (or at minimum FC.CONFIG.flightControllerVersion)
before tests run and restore it in the shared beforeEach or an afterEach so each
test (including "resource strategy" and "resource fallback") gets a fresh
config; locate FC.CONFIG references in the test file and add code to capture the
original value and reassign it back after each test.

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

Projects

Status: App

Development

Successfully merging this pull request may close these issues.

1 participant