fix(script): align check, wait and wait_check comparison parsing by jmthomas · Pull Request #3832 · OpenC3/cosmos

jmthomas · 2026-09-04T21:38:05Z

What changed

check validated the comparison operator while wait and wait_check passed the text straight to eval, so compound expressions such as "TIMEUS & 0x0001 == 0x0000" silently worked in two of the three APIs. All three now share one parser and comparator, and eval is gone from the comparison path entirely (check_expression and friends still use it by design). Operands are parsed as literals, which adds hex, octal, binary, underscore separators and recursive list elements, and rejects ambiguous or unparsable forms up front instead of after a timeout.

Why it changed

Closes #3802 which was the different behavior between wait and wait_check. This also closes potential security escapes by not using eval.

Testing strategy

Added a bunch new unit tests. Ran various script code in COSMOS 7.3 and compared output to this version.

🤖 Generated with Claude Code

check validated the comparison operator while wait and wait_check passed the text straight to eval, so compound expressions such as "TIMEUS & 0x0001 == 0x0000" silently worked in two of the three APIs. All three now share one parser and comparator, and eval is gone from the comparison path entirely (check_expression and friends still use it by design). Operands are parsed as literals, which adds hex, octal, binary, underscore separators and recursive list elements, and rejects ambiguous or unparsable forms up front instead of after a timeout. Closes #3802 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

codecov · 2026-09-04T21:40:11Z

Codecov Report

❌ Patch coverage is 95.00000% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.33%. Comparing base (95b6764) to head (2c4fb73).
⚠️ Report is 42 commits behind head on main.

Files with missing lines	Patch %	Lines
openc3/lib/openc3/script/extract.rb	95.23%	5 Missing ⚠️
openc3/lib/openc3/script/api_shared.rb	94.28%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3832      +/-   ##
==========================================
+ Coverage   79.27%   79.33%   +0.06%     
==========================================
  Files         895      896       +1     
  Lines       67271    67467     +196     
  Branches     2641     2659      +18     
==========================================
+ Hits        53327    53527     +200     
+ Misses      13273    13272       -1     
+ Partials      671      668       -3

Flag	Coverage Δ
frontend	`66.54% <ø> (+0.32%)`	⬆️
python	`79.35% <ø> (-0.02%)`	⬇️
ruby-api	`82.52% <ø> (+0.03%)`	⬆️
ruby-backend	`84.61% <95.00%> (+0.03%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

🟡 Changes recommended

There’s a confirmed edge-case bug in Ruby double-quoted \\u{...} escape handling (empty codepoint list) and a documentation example that is misleading for Python users.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR standardizes how check(), wait(), and wait_check() parse and evaluate comparison expressions across Ruby and Python, removing eval from the comparison path to eliminate inconsistent behavior and reduce security risk. It introduces shared literal operand parsing (including numeric bases, underscores, and list literals), consistent operator validation, and expands unit tests and documentation accordingly.

Changes:

Replaced eval-based comparison evaluation with a shared operator+operand parser and explicit comparator in both Ruby and Python.
Expanded operand parsing to support additional literal forms (hex/octal/binary, underscores, lists, escape handling) and fail fast on invalid/ambiguous inputs.
Added/updated unit tests and docs to enforce consistent behavior across check, wait, and wait_check.

File summaries

File	Description
openc3/spec/script/extract_spec.rb	Adds Ruby unit tests for expanded operand parsing and comparison validation.
openc3/spec/script/api_shared_spec.rb	Adds Ruby API-level consistency tests across `check/wait/wait_check`.
openc3/python/test/utilities/test_extract.py	Adds Python unit tests for operand parsing and `compare_values`.
openc3/python/test/script/test_api_shared.py	Updates Python script API tests for new comparison behavior and adds consistency tests.
openc3/python/test/api/test_api_shared.py	Updates Python API tests for new comparison behavior and adds consistency tests.
openc3/python/openc3/utilities/extract.py	Implements operand parsing without `eval` and adds `compare_values`.
openc3/python/openc3/script/api_shared.py	Switches script-side `check/wait` to parser+comparator callables instead of `eval`.
openc3/python/openc3/api/api_shared.py	Switches API-side `check/wait` to parser+comparator callables instead of `eval`.
openc3/lib/openc3/script/extract.rb	Implements Ruby operand parsing (strings, escapes, lists, numeric formats) and `compare_values`.
openc3/lib/openc3/script/api_shared.rb	Switches Ruby `check/wait` implementation to parsed comparisons/callables instead of `eval`.
docs.openc3.com/docs/guides/scripting-api.md	Documents supported comparison syntax and limitations for `check`, `wait`, and `wait_check`.

Review details

Suppressed comments (2)

docs.openc3.com/docs/guides/scripting-api.md:4163

This note is shown for the Python tab too, but the interpolation example uses Ruby syntax (#{expected}) and references check() specifically in the wait() section. Consider making the example language-neutral or giving both Ruby and Python examples.

When comparing against string or state values, the value must be quoted (e.g., `== 'ON'`). An unquoted value is rejected with `Uninitialized constant ON. Did you mean 'ON' as a string?`. Quoted values follow the string literal rules of the script language, so escape sequences are processed in Ruby double quoted strings and in all Python strings. The Ruby control and meta escapes `\c`, `\C-` and `\M-` are rejected rather than silently changed, as is string interpolation (Ruby `"#{...}"`, Python f-strings) because the comparison is not evaluated as code. Interpolate in the script itself, e.g. `check("INST HEALTH_STATUS TYPE == '#{expected}'")`.

docs.openc3.com/docs/guides/scripting-api.md:4452

This note is shown for the Python tab too, but the interpolation example uses Ruby syntax (#{expected}) and references check() in the wait_check() section. Consider making the example language-neutral or giving both Ruby and Python examples.

When comparing against string or state values, the value must be quoted (e.g., `== 'ON'`). An unquoted value is rejected with `Uninitialized constant ON. Did you mean 'ON' as a string?`. Quoted values follow the string literal rules of the script language, so escape sequences are processed in Ruby double quoted strings and in all Python strings. The Ruby control and meta escapes `\c`, `\C-` and `\M-` are rejected rather than silently changed, as is string interpolation (Ruby `"#{...}"`, Python f-strings) because the comparison is not evaluated as code. Interpolate in the script itself, e.g. `check("INST HEALTH_STATUS TYPE == '#{expected}'")`.

Files reviewed: 11/11 changed files
Comments generated: 2
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

jmthomas · 2026-09-04T21:39:22Z

+    # so unlike eval it can not execute arbitrary code. It processes string escape sequences
+    # and requires a single complete literal, so "== 'a' garbage 'b'" is a syntax error.
+    try:
+        return ast.literal_eval(operand)


This made the python version a lot simpler than the Ruby version

jmthomas · 2026-09-04T21:41:18Z

+
+      operator, operand = extract_operator_and_operand_from_comparison(comparison_to_eval)
+      return nil unless operator
+      lambda { |value| compare_values(value, operator, operand) }


lambda is a type of proc which is callable

jmthomas · 2026-09-04T21:42:53Z

+    operator, operand = extract_operator_and_operand_from_comparison(comparison_to_eval)
+    if operator is None:
+        return None
+    return lambda value: compare_values(value, operator, operand)


Similar lambda in python

jmthomas · 2026-09-04T21:44:12Z

+    rescue ArgumentError, NoMethodError, TypeError
+      # Comparing incompatible types, e.g. nil > 1, is simply not a match
+      false
    end


Ruby compare_values switch statement with all the operands we support

jmthomas · 2026-09-04T21:44:28Z

+            raise RuntimeError(f"ERROR: Invalid operator: '{operator}'")
+    except TypeError:
+        # Comparing incompatible types, e.g. None > 1, is simply not a match
+        return False


Python compare_values switch statement with all the operands we support

sonarqubecloud · 2026-09-04T22:06:37Z

Quality Gate failed

Failed conditions
28.8% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

jmthomas · 2026-09-04T23:20:20Z

SonarCloud is complaining about duplication between Ruby and Python

jmthomas requested review from ryanmelt and a lite review from Copilot September 4, 2026 21:38

github-code-quality Bot found potential problems Sep 4, 2026

View reviewed changes

Comment thread openc3/lib/openc3/script/api_shared.rb Fixed

Copilot started reviewing on behalf of jmthomas September 4, 2026 21:40 View session

Copilot AI reviewed Sep 4, 2026

View reviewed changes

Comment thread openc3/lib/openc3/script/extract.rb Outdated

Comment thread docs.openc3.com/docs/guides/scripting-api.md Outdated

jmthomas commented Sep 4, 2026

View reviewed changes

Address copilot comments

2c4fb73

Navigation Menu

Uh oh!

fix(script): align check, wait and wait_check comparison parsing - #3832

fix(script): align check, wait and wait_check comparison parsing#3832
jmthomas wants to merge 2 commits into
mainfrom
fix/check-wait-comparison-consistency

jmthomas commented Sep 4, 2026

Uh oh!

Uh oh!

codecov Bot commented Sep 4, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

jmthomas Sep 4, 2026

Uh oh!

jmthomas Sep 4, 2026

Uh oh!

jmthomas Sep 4, 2026

Uh oh!

jmthomas Sep 4, 2026

Uh oh!

jmthomas Sep 4, 2026

Uh oh!

sonarqubecloud Bot commented Sep 4, 2026

Uh oh!

jmthomas commented Sep 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

jmthomas commented Sep 4, 2026

What changed

Why it changed

Testing strategy

Uh oh!

Uh oh!

codecov Bot commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (2)

Uh oh!

Uh oh!

Uh oh!

jmthomas Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

jmthomas Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

jmthomas Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

jmthomas Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

jmthomas Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Sep 4, 2026

Quality Gate failed

Uh oh!

jmthomas commented Sep 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Sep 4, 2026 •

edited

Loading