Add JavaScript Programming tab with transpiler

[sensei-hacker]

Summary

Adds a new JavaScript Programming tab that allows users to write logic condtions (Programming tab) in JavaScript and transpile it to INAV's logic conditions format.

This feature is purely a Configurator enhancement - it does NOT modify the INAV firmware in any way. The transpiler converts JavaScript to the same logic conditions format that INAV already supports.

Features

• Monaco Editor with syntax highlighting and IntelliSense
• Real-time error validation with clear messages and line numbers
• Bidirectional translation - transpile JS to logic conditions, or decompile existing conditions back to JS
• Full API for flight data, RC channels, global variables, overrides, and more

Supported Operations

• Arithmetic, comparisons, and logical operators
• Math functions (min, max, sin, cos, tan, abs)
• RC channel access with state detection
• 8 global variables for persistent state
• Flight parameter overrides (VTX power, throttle, arming)
• Edge detection, timers, sticky conditions, and delays

Example

const { flight, override } = inav;

// Increase VTX power when far from home
if (flight.homeDistance > 500) {
  override.vtx.power = 4;
}

Screenshots

Documentation

• Wiki: JavaScript Programming Guide
• Detailed docs: inav/docs/javascript_programming/

There is also a test suite included. That accounts for most of the files. :)

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Insecure file:// loading Description: The Monaco loader builds a data: URL that injects an importScripts('file://.../workerMain.js') into a web worker and loads additional scripts from the local filesystem using file:// URLs, which can enable arbitrary local file access/code execution in the renderer context when the app runs with relaxed Electron/webview settings or nodeIntegration, exposing a clear LFI/RCE surface. javascript_programming.js [120-205] Referred Code console.log('Loading Monaco from:', vsPath); // Method 1: Try loading editor.main.js directly const editorScript = document.createElement('script'); editorScript.src = 'file://' + editorMainPath.replace(/\\/g, '/'); editorScript.onerror = () => { // Method 2: If direct load fails, try AMD loader with workaround console.log('Direct load failed, trying AMD loader...'); this.loadMonacoViaAMD(vsPath, resolve, reject); }; editorScript.onload = () => { if (window.monaco) { console.log('Monaco loaded via direct script'); resolve(window.monaco); } else { this.loadMonacoViaAMD(vsPath, resolve, reject); } }; ... (clipped 65 lines)
	Unsandboxed local script load Description: The code attempts to load Monaco by creating a <script> tag whose src points to a local file path via 'file://' concatenation of resolved Node paths, which bypasses CSP and trusts local filesystem content; if an attacker can influence the node_modules path or supply malicious files, this may lead to arbitrary script execution. javascript_programming.js [121-146] Referred Code // Method 1: Try loading editor.main.js directly const editorScript = document.createElement('script'); editorScript.src = 'file://' + editorMainPath.replace(/\\/g, '/'); editorScript.onerror = () => { // Method 2: If direct load fails, try AMD loader with workaround console.log('Direct load failed, trying AMD loader...'); this.loadMonacoViaAMD(vsPath, resolve, reject); }; editorScript.onload = () => { if (window.monaco) { console.log('Monaco loaded via direct script'); resolve(window.monaco); } else { this.loadMonacoViaAMD(vsPath, resolve, reject); } }; document.head.appendChild(editorScript); ... (clipped 5 lines)
	InnerHTML injection risk Description: Transpilation errors are inserted into the DOM using template literals without strict HTML templating; while escapeHtml is used for the message, the surrounding HTML is constructed as a string and later injected via innerHTML, increasing the risk that any missed unescaped field in future changes could enable XSS—consider using DOM construction APIs instead of innerHTML. javascript_programming.js [698-708] Referred Code showError: function(message) { const warningsDiv = $('#transpiler-warnings'); warningsDiv.html(` <div class="note error"> <div class="note_spacer"> <strong>Transpilation Error:</strong><br> ${this.escapeHtml(message)} </div> </div> `); warningsDiv.show();
	Insufficient input validation Description: The parser writes transpiled CLI commands back into the in-memory FC structure by parsing space-delimited strings ('logic ...') and pushing them without validating bounds/types beyond basic parseInt; malformed commands could corrupt state or exceed expected ranges, suggesting the need for strict validation and limits (e.g., numeric ranges, operand types, operation enums). javascript_programming.js [930-979] Referred Code } // Confirm save const confirmMsg = i18n.getMessage('confirmSaveLogicConditions') || `Save ${result.logicConditionCount} logic conditions to flight controller?`; if (!confirm(confirmMsg)) { return; } GUI.log(i18n.getMessage('savingToFC') || 'Saving to flight controller...'); // Store variable map for preservation between sessions if (result.variableMap) { settingsCache.set('javascript_variables', result.variableMap); console.log('Variable map stored:', result.variableMap); } // Clear existing logic conditions FC.LOGIC_CONDITIONS.flush(); // Parse and load transpiled commands ... (clipped 29 lines)
	Risky data: worker URL Description: The AMD loader for Monaco sets window.MonacoEnvironment.getWorkerUrl to return a data: URL containing interpolated vsPath; data: workers can be blocked by CSP and, if CSP allows them, can facilitate code execution vectors—prefer blob: with explicit content and enforce CSP restrictions. javascript_programming.js [155-166] Referred Code self.MonacoEnvironment = { baseUrl: 'file://${vsPath.replace(/\\/g, '/')}' }; importScripts('file://${vsPath.replace(/\\/g, '/')}/base/worker/workerMain.js'); `)}`; } }; const loaderScript = document.createElement('script'); loaderScript.src = 'file://' + vsPath.replace(/\\/g, '/') + '/loader.js'; loaderScript.onerror = () => {
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Missing Auditing: New actions that load/save/decompile/transpile logic conditions and write to the FC are not accompanied by audit logging specifying user, action, outcome, and timestamp. Referred Code /** * Transpile JavaScript to INAV logic conditions */ transpileCode: function() { const self = this; const code = this.editor.getValue(); if (!this.transpiler) { GUI.log(i18n.getMessage('transpilerNotAvailable') || 'Transpiler not available'); return; } GUI.log(i18n.getMessage('transpiling') || 'Transpiling JavaScript...'); try { const result = this.transpiler.transpile(code); if (result.success) { // Display output const formattedOutput = this.transpiler.formatOutput( result.commands, ... (clipped 360 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Partial Handling: While many operations catch and display errors, critical flows like saveToFC and loadFromFC lack explicit handling for MSP failures and boundary validation feedback beyond simple GUI logs. Referred Code loadFromFC: function(callback) { const self = this; if (!this.decompiler) { GUI.log(i18n.getMessage('decompilerNotAvailable') || 'Decompiler not available'); if (callback) callback(); return; } GUI.log(i18n.getMessage('loadingFromFC') || 'Loading logic conditions from flight controller...'); // Create MSP chainer for loading logic conditions const loadChainer = new MSPChainerClass(); loadChainer.setChain([ mspHelper.loadLogicConditions ]); loadChainer.setExitPoint(function() { self.onLogicConditionsLoaded(callback); }); ... (clipped 205 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Console Details: User-visible UI logs are generally generic, but several console.error messages surface internal details that could be exposed in packaged builds (e.g., Monaco load paths and stack traces). Referred Code .catch(function(error) { console.error('Failed to load Monaco Editor:', error); GUI.content_ready(callback); }); }); }, /** * Initialize transpiler and decompiler */ initTranspiler: function() { if (typeof Transpiler !== 'undefined') { this.transpiler = new Transpiler(); } else { console.error('Transpiler not loaded'); GUI.log(i18n.getMessage('transpilerNotAvailable') || 'JavaScript transpiler not available'); } if (typeof Decompiler !== 'undefined') { ... (clipped 83 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: External Inputs: The feature accepts arbitrary JavaScript and renders transpiler/decompiler messages into the DOM; although escapeHtml is used for warnings/errors, broader validation of inputs and MSP data boundaries is not fully evident. Referred Code /** * Transpile JavaScript to INAV logic conditions */ transpileCode: function() { const self = this; const code = this.editor.getValue(); if (!this.transpiler) { GUI.log(i18n.getMessage('transpilerNotAvailable') || 'Transpiler not available'); return; } GUI.log(i18n.getMessage('transpiling') || 'Transpiling JavaScript...'); try { const result = this.transpiler.transpile(code); if (result.success) { // Display output const formattedOutput = this.transpiler.formatOutput( result.commands, ... (clipped 120 lines) Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Latest suggestions up to 097fa09

Category	Suggestion	Impact
Possible issue	Align supported handlers with parser Add timer and whenChanged to the validHandlers array in checkEventHandler to align with the handlers supported by the parser. js/transpiler/transpiler/analyzer.js [259-280] checkEventHandler(stmt) { const line = stmt.loc ? stmt.loc.start.line : 0; - // Check if handler is supported - const validHandlers = ['on.arm', 'on.always', 'ifthen', 'edge', 'sticky', 'delay']; + // Check if handler is supported (must mirror parser-supported helpers) + const validHandlers = ['on.arm', 'on.always', 'ifthen', 'edge', 'sticky', 'delay', 'timer', 'whenChanged']; if (!validHandlers.includes(stmt.handler)) { this.addError(`Unknown event handler: ${stmt.handler}. Valid handlers: ${validHandlers.join(', ')}`, line); } // Check condition in 'ifthen' statements (if/else) if (stmt.condition) { this.checkCondition(stmt.condition, line); } // Check body statements if (stmt.body && Array.isArray(stmt.body)) { for (const bodyStmt of stmt.body) { this.analyzeStatement(bodyStmt); } } } Apply / Chat Suggestion importance[1-10]: 8 __ Why: This is a valid bug fix. The suggestion correctly identifies that the analyzer is missing handlers (timer, whenChanged) that the parser supports, which would cause valid code to be incorrectly rejected.	Medium
	Resolve armed operand robustly Refactor the on.arm handler to dynamically resolve the 'armed' operand using the API mapping instead of a hardcoded value, and add error handling for when the operand is not found. js/transpiler/transpiler/codegen.js [227-246] generateOnArm(stmt) { - const delay = stmt.config.delay || 0; + const delay = stmt.config?.delay ?? 0; - // Create condition: armTimer > 0 (or flight.isArmed if available) - const conditionId = this.pushLogicCommand(OPERATION.GREATER_THAN, - { type: OPERAND_TYPE.FLIGHT, value: 0 }, - { type: OPERAND_TYPE.VALUE, value: 0 } + // Resolve an explicit "armed" indicator from operand mapping + const armedKeyCandidates = [ + 'flight.mode.armed', + 'flight.armed' + ]; + let armedOperand = null; + for (const key of armedKeyCandidates) { + if (this.operandMapping[key]) { + armedOperand = this.operandMapping[key]; + break; + } + } + + if (!armedOperand) { + this.errorHandler.addError( + 'on.arm requires an armed flight operand (e.g., flight.mode.armed) but it was not found in API mapping.', + stmt, + 'missing_armed_operand' + ); + return; + } + + // condition: armed === 1 + const conditionId = this.pushLogicCommand( + OPERATION.EQUAL, + armedOperand, + { type: OPERAND_TYPE.VALUE, value: 1 } ); - // Create EDGE operation (triggers once) - const edgeId = this.pushLogicCommand(OPERATION.EDGE, + // EDGE operation (triggers once) + const edgeId = this.pushLogicCommand( + OPERATION.EDGE, { type: OPERAND_TYPE.LC, value: conditionId }, { type: OPERAND_TYPE.VALUE, value: delay } ); // Generate body actions with EDGE as activator for (const action of stmt.body) { this.generateAction(action, edgeId); } } Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that hardcoding the operand for the arming state is brittle and proposes a more robust solution using the API mapping, which improves maintainability and prevents future bugs.	Medium
	Fix boolean literal detection In isAlwaysTrue and isAlwaysFalse, refine the logic to check for boolean literals by verifying condition.type === 'Literal' instead of just checking for the string values 'true' or 'false'. js/transpiler/transpiler/optimizer.js [222-276] isAlwaysTrue(condition) { if (condition.type === 'BinaryExpression') { - // Check for things like: 1 === 1, 100 > 50 if (typeof condition.left === 'number' && typeof condition.right === 'number') { switch (condition.operator) { case '>': return condition.left > condition.right; case '<': return condition.left < condition.right; case '===': return condition.left === condition.right; case '==': return condition.left == condition.right; case '>=': return condition.left >= condition.right; case '<=': return condition.left <= condition.right; } } } - if (condition.value === 'true' || condition.value === true) { + // Only treat boolean literals as always true + if (condition.type === 'Literal' && condition.value === true) { return true; } - // Handle numeric truthiness (non-zero numbers are truthy) + // Numeric truthiness (non-zero numbers are truthy) if (condition.type === 'Literal' && typeof condition.value === 'number') { return condition.value !== 0; } return false; } isAlwaysFalse(condition) { if (condition.type === 'BinaryExpression') { if (typeof condition.left === 'number' && typeof condition.right === 'number') { switch (condition.operator) { case '>': return !(condition.left > condition.right); case '<': return !(condition.left < condition.right); case '===': return !(condition.left === condition.right); case '==': return !(condition.left == condition.right); case '>=': return !(condition.left >= condition.right); case '<=': return !(condition.left <= condition.right); } } } - if (condition.value === 'false' || condition.value === false) { + // Only treat boolean literals as always false + if (condition.type === 'Literal' && condition.value === false) { return true; } - // Handle numeric falsiness (0 is falsy) + // Numeric falsiness (0 is falsy) if (condition.type === 'Literal' && typeof condition.value === 'number') { return condition.value === 0; } return false; } Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that checking condition.value === 'true' is incorrect for the AST structure, as it could misinterpret a string. The proposed change to check condition.type === 'Literal' and condition.value === true is more robust and prevents potential optimization bugs.	Medium
	Align ESM/CJS export compatibility Align the module export format in generate-constants.js to produce an ES module, matching how inav_constants.js is authored and consumed, to prevent import failures. js/transpiler/transpiler/inav_constants.js [343-360] export { OPERAND_TYPE, OPERATION, FLIGHT_PARAM, FLIGHT_MODE, WAYPOINT_PARAM, FLIGHT_PARAM_NAMES, OPERATION_NAMES, RC_CHANNEL, GVAR_CONFIG, VTX, // Helper functions getFlightParamName, getOperationName, isValidGvarIndex, isValidRCChannel }; +// Optional: provide CommonJS default for environments requiring require() +// This keeps ESM as the primary export while avoiding runtime import errors. +// Safe no-op in strict ESM bundlers since `module` is undefined there. +/* c8 ignore next 9 */ +// eslint-disable-next-line no-undef +if (typeof module !== 'undefined' && module.exports) { + // eslint-disable-next-line no-undef + module.exports = { + OPERAND_TYPE, + OPERATION, + FLIGHT_PARAM, + FLIGHT_MODE, + WAYPOINT_PARAM, + FLIGHT_PARAM_NAMES, + OPERATION_NAMES, + RC_CHANNEL, + GVAR_CONFIG, + VTX, + getFlightParamName, + getOperationName, + isValidGvarIndex, + isValidRCChannel + }; +} + Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a mismatch between the provided inav_constants.js (ESM) and the output of generate-constants.js (CJS), which could break imports if the generator is run.	Medium
	Validate timer operand types In the decompiler, validate that TIMER operation operands are positive integer values before creating a timer() block, and add a warning if they are invalid. js/transpiler/transpiler/decompiler.js [303-316] // Check for TIMER pattern if (activator.operation === OPERATION.TIMER) { // operandA is ON duration (ms) // operandB is OFF duration (ms) // No condition - timer auto-toggles + const aTypeOk = activator.operandAType === OPERAND_TYPE.VALUE; + const bTypeOk = activator.operandBType === OPERAND_TYPE.VALUE; const onMs = activator.operandAValue; const offMs = activator.operandBValue; - return { - type: 'timer', - onMs: onMs, - offMs: offMs - }; + if (aTypeOk && bTypeOk && Number.isInteger(onMs) && Number.isInteger(offMs) && onMs > 0 && offMs > 0) { + return { + type: 'timer', + onMs: onMs, + offMs: offMs + }; + } else { + this.addWarning(`Invalid TIMER operands at index ${activator.index} - expected positive VALUE types`); + } } Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly adds validation for TIMER operands in the decompiler, making it more robust against malformed logic conditions and improving the accuracy of the decompiled code.	Low
Security	Escape warnings to prevent XSS In the showWarnings function, ensure all parts of the warning and error messages, including line numbers, are escaped using escapeHtml to prevent potential XSS vulnerabilities. tabs/javascript_programming.js [369-393] -// Show transpilation error -showError: function(message) { +// Show transpilation/decompilation warnings +showWarnings: function(warnings) { const warningsDiv = $('#transpiler-warnings'); - warningsDiv.html(` - <div class="note error"> - <div class="note_spacer"> - <strong>Transpilation Error:</strong><br> - ${this.escapeHtml(message)} - </div> - </div> - `); + let html = ''; + + if (warnings.errors && warnings.errors.length > 0) { + html += '<div class="note error"><div class="note_spacer">'; + html += '<strong>Errors:</strong><ul style="margin: 5px 0; padding-left: 20px;">'; + warnings.errors.forEach(w => { + const msg = this.escapeHtml(w.message || String(w)); + const lineInfo = w.line ? ` (line ${this.escapeHtml(String(w.line))})` : ''; + html += `<li>${msg}${lineInfo}</li>`; + }); + html += '</ul></div></div>'; + } + + if (warnings.warnings && warnings.warnings.length > 0) { + html += '<div class="note warning"><div class="note_spacer">'; + html += '<strong>Warnings:</strong><ul style="margin: 5px 0; padding-left: 20px;">'; + warnings.warnings.forEach(w => { + const msg = this.escapeHtml(w.message || String(w)); + const lineInfo = w.line ? ` (line ${this.escapeHtml(String(w.line))})` : ''; + html += `<li>${msg}${lineInfo}</li>`; + }); + html += '</ul></div></div>'; + } + + warningsDiv.html(html); warningsDiv.show(); }, [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 8 __ Why: This suggestion correctly identifies a potential XSS vulnerability where unescaped error messages could be injected into the DOM and provides a fix, which is a critical security improvement.	Medium
General	Validate update-expression targets In transformUpdateExpression, add validation to ensure the target of an update expression (++ or --) is an assignable identifier or gvar before transforming it into an assignment. js/transpiler/transpiler/parser.js [292-309] transformUpdateExpression(expr, loc, range) { if (!expr.argument) return null; const target = this.extractIdentifier(expr.argument); + + // Validate assignable target: simple identifier or gvar[n] + const isIdentifier = typeof target === 'string' && /^[A-Za-z_$][A-Za-z0-9_$]*$/.test(target); + const gvarMatch = typeof target === 'string' && /^gvar\[(\d+)\]$/.test(target); + if (!isIdentifier && !gvarMatch) { + this.addWarning('warning', `Invalid update target '${target}'. Only variables and gvar[n] can be updated.`, loc ? loc.start.line : 0); + return null; + } // Convert ++ to +1, -- to -1 const operation = expr.operator === '++' ? '+' : '-'; return { type: 'Assignment', target, operation, left: target, right: 1, loc, range }; } Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that the parser could transform an update expression on an invalid target, leading to errors later in the pipeline. Adding validation to ensure the target is assignable makes the parser more robust and provides clearer, earlier error feedback.	Medium
	Use Blob URL for worker bootstrap Replace the data: URL for the Monaco editor worker with a Blob and URL.createObjectURL to improve robustness and compatibility with strict Content Security Policies. js/transpiler/editor/monaco_loader.js [62-71] window.MonacoEnvironment = { - getWorkerUrl: function(workerId, label) { - return `data:text/javascript;charset=utf-8,${encodeURIComponent(` - self.MonacoEnvironment = { - baseUrl: '${vsPath}' - }; - importScripts('${vsPath}/base/worker/workerMain.js'); - `)}`; + getWorkerUrl: function (workerId, label) { + const script = ` + self.MonacoEnvironment = { baseUrl: '${vsPath}' }; + importScripts('${vsPath}/base/worker/workerMain.js'); + `; + try { + const blob = new Blob([script], { type: 'text/javascript' }); + return URL.createObjectURL(blob); + } catch (e) { + return `data:text/javascript;charset=utf-8,${encodeURIComponent(script)}`; } + } }; Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly points out that using a Blob and URL.createObjectURL is more robust and compliant with stricter Content Security Policies (CSP) than using a data: URL for the Monaco worker.	Low
Learned best practice	Add DOM/AMD presence guards Guard usage of window.require and document.head append with presence checks and provide fallback rejection to avoid crashes if AMD loader or DOM APIs are unavailable. js/transpiler/editor/monaco_loader.js [80-114] const editorContainer = document.getElementById(containerId); if (!editorContainer) { throw new Error(`Monaco editor container '${containerId}' not found`); } -// ... - const loaderScript = document.createElement('script'); loaderScript.src = vsPath + '/loader.js'; -loaderScript.onerror = () => { - reject(new Error('Failed to load Monaco loader.js')); -}; +loaderScript.onerror = () => reject(new Error('Failed to load Monaco loader.js')); loaderScript.onload = () => { try { - // Configure the loader + if (!window.require || typeof window.require.config !== 'function') { + reject(new Error('AMD loader not available')); + return; + } window.require.config({ - paths: { - 'vs': vsPath - }, - 'vs/nls': { - availableLanguages: {} - } + paths: { 'vs': vsPath }, + 'vs/nls': { availableLanguages: {} } }); - - // Load the editor window.require(['vs/editor/editor.main'], function() { - // Monaco is now available as a global const monaco = window.monaco; - if (!monaco || !monaco.editor) { - console.error('Monaco object not properly initialized'); reject(new Error('Monaco editor object not found')); return; } - - console.log('Monaco loaded via AMD'); resolve(monaco); }, function(err) { - console.error('AMD require error:', err); reject(err); }); } catch (err) { reject(err); } }; -document.head.appendChild(loaderScript); +if (document && document.head) { + document.head.appendChild(loaderScript); +} else { + reject(new Error('Document head not available')); +} [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Gate DOM-dependent operations with existence checks and handle failures gracefully to prevent runtime errors in dynamic environments.	Low
More

---

Previous suggestions

Suggestions up to commit 097fa09

Category	Suggestion	Impact
Possible issue	Validate parsed logic commands In saveToFC, add strict validation when parsing transpiled logic commands to ensure all parts are present and are valid integers before loading them into FC.LOGIC_CONDITIONS to prevent corrupt writes. tabs/javascript_programming.js [603-618] // Clear existing logic conditions FC.LOGIC_CONDITIONS.flush(); +let parseError = null; +let lcIndexSeen = new Set(); + // Parse and load transpiled commands for (const cmd of result.commands) { - if (cmd.startsWith('logic ')) { - const parts = cmd.split(' '); - if (parts.length >= 9) { - const lc = { - enabled: parseInt(parts[2]), - activatorId: parseInt(parts[3]), - operation: parseInt(parts[4]), - operandAType: parseInt(parts[5]), - operandAValue: parseInt(parts[6]), - operandBType: parseInt(parts[7]), - operandBValue: parseInt(parts[8]), - flags: parts[9] ? parseInt(parts[9]) : 0, + if (!cmd.startsWith('logic ')) continue; + const parts = cmd.trim().split(/\s+/); + // Expected: logic <index> <enabled> <activatorId> <operation> <aType> <aVal> <bType> <bVal> <flags?> + if (parts.length < 9 || parts.length > 10) { + parseError = `Invalid logic command format: '${cmd}'`; + break; + } + + const [ + _logic, idxStr, enabledStr, activatorStr, operationStr, + aTypeStr, aValStr, bTypeStr, bValStr, flagsStr = '0' + ] = parts; + + const idx = parseInt(idxStr, 10); + const enabled = parseInt(enabledStr, 10); + const activatorId = parseInt(activatorStr, 10); + const operation = parseInt(operationStr, 10); + const operandAType = parseInt(aTypeStr, 10); + const operandAValue = parseInt(aValStr, 10); + const operandBType = parseInt(bTypeStr, 10); + const operandBValue = parseInt(bValStr, 10); + const flags = parseInt(flagsStr, 10); + + const allNums = [idx, enabled, activatorId, operation, operandAType, operandAValue, operandBType, operandBValue, flags]; + if (allNums.some(n => !Number.isInteger(n))) { + parseError = `Non-integer field in logic command: '${cmd}'`; + break; + } + + if (idx < 0 || idx > 63) { + parseError = `Logic index out of range (0-63): ${idx}`; + break; + } + + if (enabled !== 0 && enabled !== 1) { + parseError = `Enabled must be 0 or 1 at index ${idx}`; + break; + } + + if (lcIndexSeen.has(idx)) { + parseError = `Duplicate logic index detected: ${idx}`; + break; + } + lcIndexSeen.add(idx); + + const lc = { + enabled, + activatorId, + operation, + operandAType, + operandAValue, + operandBType, + operandBValue, + flags, + + // Getter methods expected by MSPHelper + getEnabled() { return this.enabled; }, + getActivatorId() { return this.activatorId; }, + getOperation() { return this.operation; }, + getOperandAType() { return this.operandAType; }, + getOperandAValue() { return this.operandAValue; }, + getOperandBType() { return this.operandBType; }, + getOperandBValue() { return this.operandBValue; }, + getFlags() { return this.flags; } + }; + + FC.LOGIC_CONDITIONS.put(lc); +} + +if (parseError) { + this.showError(parseError); + GUI.log('Aborting save due to invalid logic command.'); + return; +} + Suggestion importance[1-10]: 9 __ Why: This suggestion addresses a critical issue where malformed transpiler output could lead to corrupt data being written to the flight controller. The proposed validation significantly improves the safety and reliability of the saveToFC function.	High
	Guard LC reference detection In groupConditions, validate that operands for EDGE, STICKY, and DELAY operations are of type LC before adding them to the referencedBySpecialOps set to prevent misclassification. js/transpiler/transpiler/decompiler.js [362-459] const groups = []; const processed = new Set(); const referencedBySpecialOps = new Set(); // First pass: find conditions referenced by EDGE/STICKY/DELAY or as LC operands const referencedAsOperand = new Set(); for (const lc of conditions) { - // Skip gap markers in first pass - if (lc._gap) continue; + // Skip gap markers in first pass + if (lc._gap) continue; - if (lc.operation === OPERATION.EDGE || - lc.operation === OPERATION.DELAY) { - referencedBySpecialOps.add(lc.operandAValue); // operandA points to condition - } else if (lc.operation === OPERATION.STICKY) { - referencedBySpecialOps.add(lc.operandAValue); // ON condition - referencedBySpecialOps.add(lc.operandBValue); // OFF condition + // Only record references when the operand types are LC + if (lc.operation === OPERATION.EDGE || lc.operation === OPERATION.DELAY) { + if (lc.operandAType === OPERAND_TYPE.LC) { + referencedBySpecialOps.add(lc.operandAValue); // operandA points to condition } - - // Track LCs referenced as operands (operandType 4 = LC result) + } else if (lc.operation === OPERATION.STICKY) { if (lc.operandAType === OPERAND_TYPE.LC) { - referencedAsOperand.add(lc.operandAValue); + referencedBySpecialOps.add(lc.operandAValue); // ON condition } if (lc.operandBType === OPERAND_TYPE.LC) { - referencedAsOperand.add(lc.operandBValue); + referencedBySpecialOps.add(lc.operandBValue); // OFF condition } + } + + // Track LCs referenced as operands (only when the operand type is LC) + if (lc.operandAType === OPERAND_TYPE.LC) { + referencedAsOperand.add(lc.operandAValue); + } + if (lc.operandBType === OPERAND_TYPE.LC) { + referencedAsOperand.add(lc.operandBValue); + } } - for (const lc of conditions) { // Handle gap markers - insert blank line group if (lc._gap) { groups.push({ _gap: true }); continue; } if (processed.has(lc.index)) continue; - // Skip conditions only used by special operations + // Skip conditions only used by special operations (referenced as LC and rootless) if (referencedBySpecialOps.has(lc.index) && lc.activatorId === -1) { - processed.add(lc.index); - continue; + processed.add(lc.index); + continue; } // Skip intermediate calculations (LCs only referenced as operands by other LCs) if (referencedAsOperand.has(lc.index) && lc.activatorId === -1 && !this.isActionOperation(lc.operation)) { - processed.add(lc.index); - continue; + processed.add(lc.index); + continue; } // Root condition (activatorId === -1) if (lc.activatorId === -1) { // Check if this is an unconditional action (e.g., var initialization, top-level assignment) if (this.isActionOperation(lc.operation)) { // Skip var initializations - they're already shown in the declarations section if (lc.operation === OPERATION.GVAR_SET && this.isVarInitialization(lc)) { processed.add(lc.index); continue; } // Treat as unconditional action - no activator, just the action itself groups.push({ activator: null, actions: [lc], isUnconditional: true }); processed.add(lc.index); continue; } const group = { activator: lc, actions: [] }; processed.add(lc.index); // Recursively find ALL descendants (not just direct children) // This handles chains like: LC0 → LC1 → LC2 → LC3 const descendants = this.collectDescendants(lc.index, conditions, processed); group.actions = descendants; groups.push(group); } } // Handle orphaned actions (actions without root conditions) for (const lc of conditions) { if (!processed.has(lc.index)) { this.addWarning(`Logic condition ${lc.index} has no valid activator`); groups.push({ activator: null, actions: [lc] }); processed.add(lc.index); } } return groups; Suggestion importance[1-10]: 8 __ Why: This is a valid bug fix that prevents the decompiler from incorrectly classifying logic conditions, which could lead to missing code in the output.	Medium
	Sync allowed event handlers Add timer and whenChanged to the validHandlers array in checkEventHandler to align with the parser's supported handlers and prevent incorrect "Unknown event handler" errors. js/transpiler/transpiler/analyzer.js [259-280] checkEventHandler(stmt) { const line = stmt.loc ? stmt.loc.start.line : 0; - // Check if handler is supported - const validHandlers = ['on.arm', 'on.always', 'ifthen', 'edge', 'sticky', 'delay']; + // Check if handler is supported (must mirror parser-supported helpers) + const validHandlers = ['on.arm', 'on.always', 'ifthen', 'edge', 'sticky', 'delay', 'timer', 'whenChanged']; if (!validHandlers.includes(stmt.handler)) { this.addError(`Unknown event handler: ${stmt.handler}. Valid handlers: ${validHandlers.join(', ')}`, line); } // Check condition in 'ifthen' statements (if/else) if (stmt.condition) { this.checkCondition(stmt.condition, line); } // Check body statements if (stmt.body && Array.isArray(stmt.body)) { for (const bodyStmt of stmt.body) { this.analyzeStatement(bodyStmt); } } } Suggestion importance[1-10]: 8 __ Why: This suggestion fixes a clear bug where the analyzer would incorrectly reject valid event handlers (timer, whenChanged) that are explicitly supported by the parser, preventing users from using these features.	Medium
	Validate DELTA operand types In detectSpecialPattern, add validation to ensure that for a DELTA operation, operandBType is VALUE and operandAType is a valid monitored type before treating it as a whenChanged pattern. js/transpiler/transpiler/decompiler.js [318-330] // Check for DELTA (whenChanged) pattern if (activator.operation === OPERATION.DELTA) { // operandA is the value to monitor - // operandB is the threshold - const valueOperand = this.decompileOperand(activator.operandAType, activator.operandAValue, allConditions); - const threshold = activator.operandBValue; + // operandB is the threshold (must be VALUE) + const validMonitoredTypes = new Set([ + OPERAND_TYPE.LC, + OPERAND_TYPE.FLIGHT, + OPERAND_TYPE.WAYPOINTS, + OPERAND_TYPE.GVAR, + OPERAND_TYPE.RC_CHANNEL, + OPERAND_TYPE.VALUE, + OPERAND_TYPE.FLIGHT_MODE + ]); - return { - type: 'whenChanged', - value: valueOperand, - threshold: threshold - }; + if (validMonitoredTypes.has(activator.operandAType) && activator.operandBType === OPERAND_TYPE.VALUE) { + const valueOperand = this.decompileOperand( + activator.operandAType, + activator.operandAValue, + allConditions + ); + const threshold = activator.operandBValue; + + return { + type: 'whenChanged', + value: valueOperand, + threshold: threshold + }; + } + // Fallback: don't mislabel an unknown DELTA usage + // Let normal condition flow handle it elsewhere } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that the DELTA pattern detection is missing operand type validation, which could lead to incorrect decompilation. The fix improves the decompiler's robustness.	Medium
	Robust gvar read extraction Improve findGvarReads to recursively traverse all relevant AST nodes and extract numeric gvar indices, ensuring all reads in conditions are detected. js/transpiler/transpiler/analyzer.js [623-651] findGvarReads(condition, used) { if (!condition) return; - + + const toGvarIndex = (str) => { + const m = typeof str === 'string' ? str.match(/^gvar\[(\d+)\]$/) : null; + return m ? parseInt(m[1], 10) : null; + }; + + // Check direct string payload (some transformed nodes store string in value/left/right) + const checkValue = (val) => { + const idx = toGvarIndex(val); + if (idx !== null) used.add(idx); + }; + switch (condition.type) { case 'BinaryExpression': - if (typeof condition.left === 'string' && condition.left.startsWith('gvar[')) { - used.add(condition.left); - } - if (typeof condition.right === 'string' && condition.right.startsWith('gvar[')) { - used.add(condition.right); - } + case 'LogicalExpression': + // left/right may be AST nodes or strings + if (condition.left) this.findGvarReads(condition.left, used); + if (condition.right) this.findGvarReads(condition.right, used); + checkValue(condition.left); + checkValue(condition.right); break; - + + case 'UnaryExpression': + if (condition.argument) this.findGvarReads(condition.argument, used); + checkValue(condition.argument); + break; + case 'MemberExpression': - if (typeof condition.value === 'string' && condition.value.startsWith('gvar[')) { - used.add(condition.value); - } + // our simplified AST stores property path in .value + checkValue(condition.value); break; - - case 'LogicalExpression': - case 'UnaryExpression': - this.findGvarReads(condition.left, used); - this.findGvarReads(condition.right, used); - if (condition.argument) { - this.findGvarReads(condition.argument, used); - } + + case 'Identifier': + case 'Literal': + checkValue(condition.value); + break; + + default: + // Best-effort: inspect common fields if present + if (condition.left) this.findGvarReads(condition.left, used); + if (condition.right) this.findGvarReads(condition.right, used); + if (condition.argument) this.findGvarReads(condition.argument, used); + if (condition.value) checkValue(condition.value); break; } } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that findGvarReads is incomplete and could miss gvar usages in conditions, leading to incorrect analysis. The proposed change makes the traversal more comprehensive and robust.	Medium
	Validate override targets before emitting Add a validation check in generateOverride to ensure getOverrideOperation returns a valid operation before creating a logic command, and report an error if the target is invalid. js/transpiler/transpiler/action_generator.js [193-202] generateOverride(target, value, activatorId) { const operation = this.getOverrideOperation(target); + if (operation === null || typeof operation === 'undefined') { + this.errorHandler.addError( + `Unknown or unsupported override target '${target}'`, + null, + 'invalid_override_target' + ); + return; + } + const valueOperand = this.getOperand(value); - this.pushLogicCommand(operation, + this.pushLogicCommand( + operation, valueOperand, - { type: 0, value: 0 }, + { type: OPERAND_TYPE.VALUE, value: 0 }, activatorId ); } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a missing validation check that could lead to generating invalid logic commands if an unsupported override target is used, improving the transpiler's robustness.	Medium
	Normalize gvar tracking to indices Refactor detectUninitializedGvars to track gvar usage by their numeric index instead of by string representation (gvar[0]) to improve robustness. js/transpiler/transpiler/analyzer.js [576-618] detectUninitializedGvars(ast) { - const initialized = new Set(); - const used = new Set(); - + const initialized = new Set(); // holds numeric indices + const used = new Set(); // holds numeric indices + + const toGvarIndex = (str) => { + const m = typeof str === 'string' ? str.match(/^gvar\[(\d+)\]$/) : null; + return m ? parseInt(m[1], 10) : null; + }; + for (const stmt of ast.statements) { if (stmt && stmt.type === 'EventHandler') { // Track which gvars are written if (stmt.body && Array.isArray(stmt.body)) { for (const bodyStmt of stmt.body) { if (bodyStmt && bodyStmt.type === 'Assignment') { - if (bodyStmt.target.startsWith('gvar[')) { - initialized.add(bodyStmt.target); + const tgtIdx = toGvarIndex(bodyStmt.target); + if (tgtIdx !== null) { + initialized.add(tgtIdx); } // Check if right side uses gvars - if (typeof bodyStmt.value === 'string' && bodyStmt.value.startsWith('gvar[')) { - used.add(bodyStmt.value); + const valIdx = toGvarIndex(bodyStmt.value); + if (valIdx !== null) { + used.add(valIdx); } if (bodyStmt.operation) { - if (typeof bodyStmt.left === 'string' && bodyStmt.left.startsWith('gvar[')) { - used.add(bodyStmt.left); - } - if (typeof bodyStmt.right === 'string' && bodyStmt.right.startsWith('gvar[')) { - used.add(bodyStmt.right); - } + const leftIdx = toGvarIndex(bodyStmt.left); + if (leftIdx !== null) used.add(leftIdx); + const rightIdx = toGvarIndex(bodyStmt.right); + if (rightIdx !== null) used.add(rightIdx); } } } } - + // Track which gvars are read in conditions if (stmt.condition) { this.findGvarReads(stmt.condition, used); } } } - + // Warn about gvars used but never initialized - for (const gvar of used) { - if (!initialized.has(gvar)) { - this.addWarning('uninitialized', `${gvar} is used but never initialized. Will default to 0.`, 0); + for (const idx of used) { + if (!initialized.has(idx)) { + this.addWarning('uninitialized', `gvar[${idx}] is used but never initialized. Will default to 0.`, 0); } } } Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that tracking gvar usage by string is fragile and proposes a more robust method of using numeric indices, which improves the correctness of the uninitialized variable detection.	Low
General	Validate axis index before decompiling Add a bounds check for the axisIndex in handleFlightAxisAngleOverride and handleFlightAxisRateOverride to prevent generating invalid code from out-of-range axis values. js/transpiler/transpiler/action_decompiler.js [215-231] handleFlightAxisAngleOverride(lc, value) { - // operandA is axis (0=roll, 1=pitch, 2=yaw), operandB is angle in degrees const axisNames = ['roll', 'pitch', 'yaw']; const axisIndex = lc.operandAValue; - const axisName = axisNames[axisIndex] || axisIndex; + if (axisIndex < 0 || axisIndex > 2) { + this.addWarning(`FLIGHT_AXIS_ANGLE_OVERRIDE with invalid axis index ${axisIndex}`); + return `/* invalid axis index ${axisIndex} for flightAxis.angle */`; + } + const axisName = axisNames[axisIndex]; this.addWarning(`FLIGHT_AXIS_ANGLE_OVERRIDE may need verification - check API syntax`); return `override.flightAxis.${axisName}.angle = ${value};`; } handleFlightAxisRateOverride(lc, value) { - // operandA is axis (0=roll, 1=pitch, 2=yaw), operandB is rate in deg/s const axisNames = ['roll', 'pitch', 'yaw']; const axisIndex = lc.operandAValue; - const axisName = axisNames[axisIndex] || axisIndex; + if (axisIndex < 0 || axisIndex > 2) { + this.addWarning(`FLIGHT_AXIS_RATE_OVERRIDE with invalid axis index ${axisIndex}`); + return `/* invalid axis index ${axisIndex} for flightAxis.rate */`; + } + const axisName = axisNames[axisIndex]; this.addWarning(`FLIGHT_AXIS_RATE_OVERRIDE may need verification - check API syntax`); return `override.flightAxis.${axisName}.rate = ${value};`; } Suggestion importance[1-10]: 7 __ Why: This suggestion improves the decompiler's robustness by adding a necessary bounds check for the axis index, preventing the generation of invalid JavaScript for malformed logic conditions.	Medium
	Normalize amplitude operand to number In trigonometric decompiler functions, convert the right operand to a number before checking if it is zero to handle different representations of zero consistently. js/transpiler/transpiler/condition_decompiler.js [248-270] handleSin(left, right) { - // SIN: sin(A degrees) * B, or * 500 if B is 0 - if (right === '0') { + const amp = Number(right); + if (Number.isFinite(amp) && amp === 0) { return `(Math.sin(${left} * Math.PI / 180) * 500)`; } return `(Math.sin(${left} * Math.PI / 180) * ${right})`; } handleCos(left, right) { - // COS: cos(A degrees) * B, or * 500 if B is 0 - if (right === '0') { + const amp = Number(right); + if (Number.isFinite(amp) && amp === 0) { return `(Math.cos(${left} * Math.PI / 180) * 500)`; } return `(Math.cos(${left} * Math.PI / 180) * ${right})`; } handleTan(left, right) { - // TAN: tan(A degrees) * B, or * 500 if B is 0 - if (right === '0') { + const amp = Number(right); + if (Number.isFinite(amp) && amp === 0) { return `(Math.tan(${left} * Math.PI / 180) * 500)`; } return `(Math.tan(${left} * Math.PI / 180) * ${right})`; } Suggestion importance[1-10]: 5 __ Why: The suggestion correctly points out that comparing right as a string is not robust, but the decompileOperand function already returns a string representation, making a strict numeric check less critical in this context.	Low
Incremental [*]	Guard operand string handling Add type guards within the getOperand function to ensure the value variable is a string before string methods are called on it, preventing potential runtime errors from unexpected return values from variableHandler. js/transpiler/transpiler/codegen.js [583-638] if (typeof value === 'string') { // Check if it's a variable reference if (this.variableHandler && this.variableHandler.isVariable(value)) { const resolution = this.variableHandler.resolveVariable(value); - if (resolution.type === 'let_expression') { + if (resolution && resolution.type === 'let_expression') { // Inline substitute the expression AST return this.getOperand(resolution.ast, activatorId); - } else if (resolution.type === 'var_gvar') { + } else if (resolution && resolution.type === 'var_gvar') { // Replace with gvar reference and continue - value = resolution.gvarRef; + value = String(resolution.gvarRef); } + } + + // Safeguard: ensure value is still a string + if (typeof value !== 'string') { + this.errorHandler.addError(`Invalid operand resolution. Expected string identifier, got ${typeof value}.`, null, 'invalid_operand_resolution'); + return { type: OPERAND_TYPE.VALUE, value: 0 }; } // Check for gvar with bounds validation if (value.startsWith('gvar[')) { const match = value.match(/^gvar\[(\d+)\]$/); if (!match) { this.errorHandler.addError(`Invalid gvar syntax '${value}'. Expected gvar[0-7].`, null, 'invalid_gvar'); return { type: OPERAND_TYPE.VALUE, value: 0 }; } const index = parseInt(match[1], 10); if (index < 0 || index > 7) { this.errorHandler.addError(`Invalid gvar index ${index}. Must be 0-7.`, null, 'invalid_gvar_index'); return { type: OPERAND_TYPE.VALUE, value: 0 }; } return { type: OPERAND_TYPE.GVAR, value: index }; } // Check for rc channel with bounds validation if (value.startsWith('rc[')) { const match = value.match(/^rc\[(\d+)\]$/); if (!match) { this.errorHandler.addError(`Invalid rc syntax '${value}'. Expected rc[1-18].`, null, 'invalid_rc'); return { type: OPERAND_TYPE.VALUE, value: 0 }; } const index = parseInt(match[1], 10); if (index < 1 || index > 18) { this.errorHandler.addError(`Invalid rc channel ${index}. Must be 1-18.`, null, 'invalid_rc_index'); return { type: OPERAND_TYPE.VALUE, value: 0 }; } return { type: OPERAND_TYPE.RC_CHANNEL, value: index }; } // Check in operand mapping if (this.operandMapping[value]) { return this.operandMapping[value]; } this.errorHandler.addError( `Unknown operand '${value}'. Available: flight.*, rc.*, gvar[0-7], waypoint.*, pid.*`, null, 'unknown_operand' ); return { type: OPERAND_TYPE.VALUE, value: 0 }; // Return dummy value to continue collecting errors } Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies a potential runtime error if variableHandler.resolveVariable returns an unexpected value, and the proposed change adds necessary type guards to make the code more robust.	Low

✅ Suggestions up to commit f22e4bd

Category	Suggestion	Impact
Possible issue	Use distinct operands for RC states Assign distinct inavOperand mappings for the low, mid, and high properties of RC channels to differentiate them from the numeric value property and ensure correct transpilation. js/transpiler/api/definitions/rc.js [19-56] for (let i = 1; i <= 18; i++) { rcChannels[i] = { type: 'object', desc: `RC channel ${i}`, readonly: true, properties: { value: { type: 'number', unit: 'us', desc: `Channel ${i} value in microseconds (1000-2000)`, readonly: true, range: [1000, 2000], - inavOperand: { type: 4, value: i } // OPERAND_RC_CHANNEL (1-based) + inavOperand: { type: 4, value: i } // e.g., OPERAND_RC_VALUE (1-based) }, - low: { type: 'boolean', desc: `Channel ${i} is in low position (< 1333us)`, readonly: true, - inavOperand: { type: 4, value: i } + inavOperand: { type: 40, value: i } // distinct mapping for LOW state }, - mid: { type: 'boolean', desc: `Channel ${i} is in middle position (1333-1666us)`, readonly: true, - inavOperand: { type: 4, value: i } + inavOperand: { type: 41, value: i } // distinct mapping for MID state }, - high: { type: 'boolean', desc: `Channel ${i} is in high position (> 1666us)`, readonly: true, - inavOperand: { type: 4, value: i } + inavOperand: { type: 42, value: i } // distinct mapping for HIGH state } } }; } Suggestion importance[1-10]: 9 __ Why: This suggestion correctly identifies a critical design flaw where different RC channel properties (value, low, mid, high) map to the same operand, making it impossible for the transpiler to distinguish them and generate correct logic.	High
	✅ Validate bracketed operand indices Suggestion Impact: The commit implemented strict regex-based parsing and range validation for gvar and rc operands, adding error handling for invalid syntax and out-of-bounds indices. code diff: - // Check for gvar + // Check for gvar with bounds validation if (value.startsWith('gvar[')) { - const index = parseInt(value.match(/\d+/)[0]); + const match = value.match(/^gvar\[(\d+)\]$/); + if (!match) { + this.errorHandler.addError(`Invalid gvar syntax '${value}'. Expected gvar[0-7].`, null, 'invalid_gvar'); + return { type: OPERAND_TYPE.VALUE, value: 0 }; + } + const index = parseInt(match[1], 10); + if (index < 0 || index > 7) { + this.errorHandler.addError(`Invalid gvar index ${index}. Must be 0-7.`, null, 'invalid_gvar_index'); + return { type: OPERAND_TYPE.VALUE, value: 0 }; + } return { type: OPERAND_TYPE.GVAR, value: index }; } - // Check for rc channel + // Check for rc channel with bounds validation if (value.startsWith('rc[')) { - const index = parseInt(value.match(/\d+/)[0]); + const match = value.match(/^rc\[(\d+)\]$/); + if (!match) { + this.errorHandler.addError(`Invalid rc syntax '${value}'. Expected rc[1-18].`, null, 'invalid_rc'); + return { type: OPERAND_TYPE.VALUE, value: 0 }; + } + const index = parseInt(match[1], 10); + if (index < 1 || index > 18) { + this.errorHandler.addError(`Invalid rc channel ${index}. Must be 1-18.`, null, 'invalid_rc_index'); + return { type: OPERAND_TYPE.VALUE, value: 0 }; + } return { type: OPERAND_TYPE.RC_CHANNEL, value: index }; Add strict validation and bounds checking for gvar and rc channel indices to prevent malformed inputs from generating corrupt commands. js/transpiler/transpiler/codegen.js [574-628] getOperand(value, activatorId = -1) { if (typeof value === 'number') { return { type: OPERAND_TYPE.VALUE, value }; } if (typeof value === 'boolean') { return { type: OPERAND_TYPE.VALUE, value: value ? 1 : 0 }; } if (typeof value === 'string') { // Check if it's a variable reference if (this.variableHandler && this.variableHandler.isVariable(value)) { const resolution = this.variableHandler.resolveVariable(value); if (resolution.type === 'let_expression') { // Inline substitute the expression AST return this.getOperand(resolution.ast, activatorId); } else if (resolution.type === 'var_gvar') { // Replace with gvar reference and continue value = resolution.gvarRef; } } - // Check for gvar + // Check for gvar[index] with strict validation if (value.startsWith('gvar[')) { - const index = parseInt(value.match(/\d+/)[0]); - return { type: OPERAND_TYPE.GVAR, value: index }; + const m = value.match(/^gvar\[(\d{1,2})\]$/); + const idx = m ? parseInt(m[1], 10) : NaN; + if (!Number.isInteger(idx) || idx < 0 || idx > 7) { + this.errorHandler.addError(`Invalid gvar index in '${value}'. Expected gvar[0-7].`, null, 'invalid_gvar_index'); + return { type: OPERAND_TYPE.VALUE, value: 0 }; + } + return { type: OPERAND_TYPE.GVAR, value: idx }; } - // Check for rc channel + // Check for rc[index] with strict validation (1-based) if (value.startsWith('rc[')) { - const index = parseInt(value.match(/\d+/)[0]); - return { type: OPERAND_TYPE.RC_CHANNEL, value: index }; + const m = value.match(/^rc\[(\d{1,2})\]$/); + const idx = m ? parseInt(m[1], 10) : NaN; + if (!Number.isInteger(idx) || idx < 1 || idx > 18) { + this.errorHandler.addError(`Invalid rc index in '${value}'. Expected rc[1-18].`, null, 'invalid_rc_index'); + return { type: OPERAND_TYPE.VALUE, value: 0 }; + } + return { type: OPERAND_TYPE.RC_CHANNEL, value: idx }; } // Check in operand mapping if (this.operandMapping[value]) { return this.operandMapping[value]; } this.errorHandler.addError( `Unknown operand '${value}'. Available: flight.*, rc.*, gvar[0-7], waypoint.*, pid.*`, null, 'unknown_operand' ); return { type: OPERAND_TYPE.VALUE, value: 0 }; // Return dummy value to continue collecting errors } // Handle expression objects (CallExpression, BinaryExpression, etc.) if (typeof value === 'object' && value !== null && value.type) { return this.generateExpression(value, activatorId); } return { type: OPERAND_TYPE.VALUE, value: 0 }; } [Suggestion processed] Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a significant flaw where malformed gvar or rc channel indices are not properly validated, potentially leading to corrupt output. The proposed change adds strict parsing and bounds checking, which is a critical improvement for correctness and error handling.	Medium
	Correct literal-only truthiness evaluation Correct the logic in isAlwaysTrue to properly inspect the AST for Literal nodes and their values, ensuring constant boolean conditions are evaluated correctly. js/transpiler/transpiler/optimizer.js [222-247] isAlwaysTrue(condition) { + if (!condition || typeof condition !== 'object') return false; + if (condition.type === 'BinaryExpression') { - // Check for things like: 1 === 1, 100 > 50 - if (typeof condition.left === 'number' && typeof condition.right === 'number') { + const leftIsNumLit = condition.left && condition.left.type === 'Literal' && typeof condition.left.value === 'number'; + const rightIsNumLit = condition.right && condition.right.type === 'Literal' && typeof condition.right.value === 'number'; + if (leftIsNumLit && rightIsNumLit) { + const L = condition.left.value; + const R = condition.right.value; switch (condition.operator) { - case '>': return condition.left > condition.right; - case '<': return condition.left < condition.right; - case '===': return condition.left === condition.right; - case '==': return condition.left == condition.right; - case '>=': return condition.left >= condition.right; - case '<=': return condition.left <= condition.right; + case '>': return L > R; + case '<': return L < R; + case '===': return L === R; + case '==': return L == R; // intentional loose check per original logic + case '>=': return L >= R; + case '<=': return L <= R; } } } - if (condition.value === 'true' || condition.value === true) { - return true; - } - - // Handle numeric truthiness (non-zero numbers are truthy) - if (condition.type === 'Literal' && typeof condition.value === 'number') { - return condition.value !== 0; + if (condition.type === 'Literal') { + if (typeof condition.value === 'boolean') return condition.value === true; + if (typeof condition.value === 'number') return condition.value !== 0; } return false; } Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a critical bug where the isAlwaysTrue function makes incorrect assumptions about the AST structure, which would cause dead code elimination to fail or behave incorrectly.	Medium
	Restrict falsiness to literal-only evaluation Correct the logic in isAlwaysFalse to properly inspect the AST for Literal nodes and their values, ensuring constant boolean conditions are evaluated correctly. js/transpiler/transpiler/optimizer.js [252-276] isAlwaysFalse(condition) { + if (!condition || typeof condition !== 'object') return false; + if (condition.type === 'BinaryExpression') { - if (typeof condition.left === 'number' && typeof condition.right === 'number') { + const leftIsNumLit = condition.left && condition.left.type === 'Literal' && typeof condition.left.value === 'number'; + const rightIsNumLit = condition.right && condition.right.type === 'Literal' && typeof condition.right.value === 'number'; + if (leftIsNumLit && rightIsNumLit) { + const L = condition.left.value; + const R = condition.right.value; switch (condition.operator) { - case '>': return !(condition.left > condition.right); - case '<': return !(condition.left < condition.right); - case '===': return !(condition.left === condition.right); - case '==': return !(condition.left == condition.right); - case '>=': return !(condition.left >= condition.right); - case '<=': return !(condition.left <= condition.right); + case '>': return !(L > R); + case '<': return !(L < R); + case '===': return !(L === R); + case '==': return !(L == R); // intentional loose check per original logic + case '>=': return !(L >= R); + case '<=': return !(L <= R); } } } - if (condition.value === 'false' || condition.value === false) { - return true; - } - - // Handle numeric falsiness (0 is falsy) - if (condition.type === 'Literal' && typeof condition.value === 'number') { - return condition.value === 0; + if (condition.type === 'Literal') { + if (typeof condition.value === 'boolean') return condition.value === false; + if (typeof condition.value === 'number') return condition.value === 0; } return false; } Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a critical bug where the isAlwaysFalse function makes incorrect assumptions about the AST structure, which would cause dead code elimination to fail or behave incorrectly.	Medium
	Preserve explicit RC override syntax In handleRcChannelOverride, decompile to an explicit override function call like override.rcChannel(...) instead of rc[N] = ... to avoid ambiguity between read-only state and writable overrides. js/transpiler/transpiler/action_decompiler.js [176-180] handleRcChannelOverride(lc, value) { // operandA contains channel number (1-based: 1-18) - // Use cleaner array syntax instead of override.rcChannel() - return `rc[${lc.operandAValue}] = ${value};`; + // Preserve override intent for round-trip correctness + return `override.rcChannel(${lc.operandAValue}, ${value});`; } Suggestion importance[1-10]: 8 __ Why: The suggestion correctly points out that decompiling an override to rc[N] = ... is misleading, as rc properties are read-only inputs. Using an explicit override syntax like override.rcChannel(...) would improve clarity and ensure correct round-trip transpilation.

[Scavanger]

#2445