Feature/enhanced workflow validation by Sahil-u07 · Pull Request #229 · ControlCore-Project/concore

Sahil-u07 · 2026-02-09T10:48:41Z

Summary

Enhanced the concore validate command with comprehensive checking capabilities to catch configuration errors before workflow execution.

New Features

Source file validation: Optional --source flag to verify referenced files exist
Cycle detection: Identifies control loops (with informative warnings)
ZMQ port validation: Detects port conflicts and reserved port usage
Port range checks: Validates ports are within valid range (1-65535)

Changes

Modified concore_cli/commands/validate.py - Added 3 helper functions
Updated concore_cli/cli.py - Added --source option
Enhanced concore_cli/README.md - Updated documentation
Expanded tests/test_graph.py - Added 6 new test cases

Testing

All 13 validation tests pass

Benefits

Catches errors earlier in the workflow
Reduces runtime failures
Improves user experience with clear error messages
Better validates ZMQ-based workflows

- Register terminate_zmq() with atexit to ensure cleanup on normal exit - Add signal handlers for SIGINT (Ctrl+C) and SIGTERM - Improve terminate_zmq() with better logging and error handling - Clear zmq_ports dict after cleanup to prevent double-cleanup

…pecific SIGTERM

- Add optional --source flag to validate command for file existence checking - Implement cycle detection to identify control loops in workflows - Add ZMQ port conflict detection and validation - Warn about reserved ports (< 1024) and invalid port ranges - Expand test coverage with 6 new comprehensive test cases - Update CLI documentation with new validation features This improves the user experience by catching common configuration errors before workflow execution, reducing runtime failures.

Copilot

Pull request overview

Enhances the concore validate CLI command to catch additional workflow configuration issues prior to execution, including optional source-file existence checks, cycle detection warnings, and ZMQ port validation.

Changes:

Add --source option to concore validate and validate referenced node source files when provided.
Add workflow cycle detection (warning) and ZMQ port conflict/reserved-range validation.
Expand graph validation test coverage to cover the new validation behaviors.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`concore_cli/commands/validate.py`	Adds optional source-file validation plus cycle and ZMQ port validation helpers.
`concore_cli/cli.py`	Adds `concore validate --source` option and wires it into `validate_workflow`.
`concore_cli/README.md`	Documents the new `validate` option and expanded validation checks.
`tests/test_graph.py`	Adds new CLI tests for source-file validation, cycle warnings, and ZMQ port checks.
`concore.py`	Adds signal handling and more robust ZMQ cleanup behavior (not described in PR summary).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-09T10:53:22Z

concore_cli/commands/validate.py

+        if port_num < 1024:
+            warnings.append(f"Port {port_num} (0x{port_hex}) is in reserved range (< 1024)")
+        elif port_num > 65535:
+            errors.append(f"Invalid port number: {port_num} (0x{port_hex}) exceeds maximum (65535)")


_check_zmq_ports() treats any port <1024 as a warning, which includes port 0. Port 0 is not a valid TCP port, and the PR description states ports must be within 1–65535. Add an explicit error for port_num < 1 (before the reserved-range warning) so invalid ports are rejected.

Copilot · 2026-02-09T10:53:22Z

concore_cli/cli.py

 @cli.command()
 @click.argument('workflow_file', type=click.Path(exists=True))
-def validate(workflow_file):
+@click.option('--source', '-s', type=click.Path(exists=True), help='Source directory to check file references')


The validate --source option uses click.Path(exists=True) but doesn’t restrict the value to a directory, so a file path would be accepted and then used as a “directory” in validation. Configure the option with file_okay=False/dir_okay=True (and optionally path_type=Path) so the CLI enforces a directory input.

Suggested change

@click.option('--source', '-s', type=click.Path(exists=True), help='Source directory to check file references')

@click.option(

'--source',

'-s',

type=click.Path(exists=True, file_okay=False, dir_okay=True, path_type=Path),

help='Source directory to check file references',

)

Copilot · 2026-02-09T10:53:22Z

concore_cli/commands/validate.py

+def _check_source_files(soup, source_path, errors, warnings):
+    nodes = soup.find_all('node')
+


_check_source_files() takes a warnings parameter but never uses it. Either remove the unused parameter (and update the call site) or add a warning for skipped/invalid node label formats so callers can understand why some nodes weren’t checked.

Copilot · 2026-02-09T10:53:23Z

concore.py

+    print("\nCleaning up ZMQ resources...")
+    for port_name, port in zmq_ports.items():
        try:
            port.socket.close()
            port.context.term()
+            print(f"Closed ZMQ port: {port_name}")
        except Exception as e:
            logging.error(f"Error while terminating ZMQ port {port.address}: {e}")
+    zmq_ports.clear()
+    _cleanup_in_progress = False

+def signal_handler(sig, frame):
+    """Handle interrupt signals gracefully."""
+    print(f"\nReceived signal {sig}, shutting down gracefully...")


terminate_zmq()/signal_handler() print directly to stdout (cleanup messages). This can pollute workflow output and makes log handling inconsistent with the rest of the file which uses logging. Prefer logging.info/warning (or gate prints behind a verbosity/TTY check) for these messages.

Copilot · 2026-02-09T10:53:23Z

concore.py

+def signal_handler(sig, frame):
+    """Handle interrupt signals gracefully."""
+    print(f"\nReceived signal {sig}, shutting down gracefully...")
+    # Prevent terminate_zmq from being called twice: once here and once via atexit
+    try:
+        atexit.unregister(terminate_zmq)
+    except Exception:
+        # If unregister fails for any reason, proceed with explicit cleanup anyway
+        pass
+    terminate_zmq()
+    sys.exit(0)
+
+# Register cleanup handlers
 atexit.register(terminate_zmq)
+signal.signal(signal.SIGINT, signal_handler)   # Handle Ctrl+C
+if not hasattr(sys, 'getwindowsversion'):
+    signal.signal(signal.SIGTERM, signal_handler)  # Handle termination (Unix only)
+


The PR description doesn’t mention changes to concore.py, but this PR adds signal handling and alters ZMQ shutdown behavior here. Either update the PR description to include this file/change or split it into a separate PR so the validate-related change set stays focused.

Copilot · 2026-02-09T10:53:23Z

tests/test_graph.py

+    def test_validate_zmq_port_conflict(self):
+        content = '''
+        <graphml xmlns:y="http://www.yworks.com/xml/graphml">
+            <graph id="G" edgedefault="directed">
+                <node id="n0">
+                    <data key="d0"><y:NodeLabel>n0:script1.py</y:NodeLabel></data>
+                </node>
+                <node id="n1">
+                    <data key="d0"><y:NodeLabel>n1:script2.py</y:NodeLabel></data>
+                </node>
+                <edge source="n0" target="n1">
+                    <data key="d1"><y:EdgeLabel>0x1234_portA</y:EdgeLabel></data>
+                </edge>
+                <edge source="n1" target="n0">
+                    <data key="d1"><y:EdgeLabel>0x1234_portB</y:EdgeLabel></data>
+                </edge>
+            </graph>
+        </graphml>
+        '''
+        filepath = self.create_graph_file('conflict.graphml', content)
+
+        result = self.runner.invoke(cli, ['validate', filepath])
+
+        self.assertIn('Validation failed', result.output)
+        self.assertIn('Port conflict', result.output)
+
+    def test_validate_reserved_port(self):
+        content = '''
+        <graphml xmlns:y="http://www.yworks.com/xml/graphml">
+            <graph id="G" edgedefault="directed">
+                <node id="n0">
+                    <data key="d0"><y:NodeLabel>n0:script1.py</y:NodeLabel></data>
+                </node>
+                <node id="n1">
+                    <data key="d0"><y:NodeLabel>n1:script2.py</y:NodeLabel></data>
+                </node>
+                <edge source="n0" target="n1">
+                    <data key="d1"><y:EdgeLabel>0x50_data</y:EdgeLabel></data>
+                </edge>
+            </graph>
+        </graphml>
+        '''
+        filepath = self.create_graph_file('reserved.graphml', content)
+
+        result = self.runner.invoke(cli, ['validate', filepath])
+
+        self.assertIn('Port 80', result.output)
+        self.assertIn('reserved range', result.output)
+


Port validation tests cover reserved-port warnings and same-port conflicts, but there’s no test for the stated port-range requirement (1–65535). After adding the <1 check (and optionally asserting the existing >65535 behavior), add tests for port 0 (should fail) and a port > 65535 (should fail) to lock in the intended behavior.

- Add explicit validation for port 0 (invalid) - Enforce directory-only input for --source option - Use warnings parameter in _check_source_files for clarity - Add test coverage for port 0 and port > 65535 edge cases - Reorder port validation to check range before conflicts

Sahil-u07 added 4 commits February 5, 2026 02:34

fix: address Copilot review, prevent reentrant cleanup and platform-s…

b3268f3

…pecific SIGTERM

Merge upstream/dev and resolve conflicts in concore.py

6355f7b

Copilot AI review requested due to automatic review settings February 9, 2026 10:48

Copilot started reviewing on behalf of Sahil-u07 February 9, 2026 10:49 View session

Copilot AI reviewed Feb 9, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/enhanced workflow validation#229

Feature/enhanced workflow validation#229
Sahil-u07 wants to merge 5 commits intoControlCore-Project:devfrom
Sahil-u07:feature/enhanced-workflow-validation

Sahil-u07 commented Feb 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		def _check_source_files(soup, source_path, errors, warnings):
		nodes = soup.find_all('node')

Conversation

Sahil-u07 commented Feb 9, 2026

Summary

New Features

Changes

Testing

Benefits

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant