Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 8, 2025

Refactors strict mode validation to follow the established *_validation.go naming convention used throughout the codebase (e.g., docker_validation.go, npm_validation.go, pip_validation.go).

Changes

  • Renamed: validation_strict_mode.gostrict_mode_validation.go for consistency with naming pattern
  • Consolidated: Moved validateStrictMode() orchestrator from strict_mode.go into validation file
  • Removed: strict_mode.go (now redundant - contained only orchestrator function)
  • Updated: Documentation references in validation.go, CONTRIBUTING.md, and specs/validation-architecture.md

Structure

All strict mode validation logic now lives in a single 161-line file:

// pkg/workflow/strict_mode_validation.go

// Individual validators
func (c *Compiler) validateStrictPermissions(frontmatter map[string]any) error
func (c *Compiler) validateStrictNetwork(networkPermissions *NetworkPermissions) error
func (c *Compiler) validateStrictMCPNetwork(frontmatter map[string]any) error

// Main orchestrator (moved from strict_mode.go)
func (c *Compiler) validateStrictMode(frontmatter map[string]any, networkPermissions *NetworkPermissions) error

Resolves #3478 (Priority 2 from semantic function clustering analysis).

Original prompt

This section details on the original issue you should resolve

<issue_title>[task] Extract validation methods from strict_mode.go to strict_mode_validation.go</issue_title>
<issue_description>## Objective
Extract validation methods from pkg/workflow/strict_mode.go into a dedicated validation file following the established *_validation.go pattern.

Context

Similar to the engine.go issue, strict_mode.go contains validation methods that should be in a dedicated validation file for consistency with the codebase pattern (docker_validation.go, npm_validation.go, etc.). This is part of Issue #3 from the semantic function clustering analysis (Priority 2: Medium Impact, Medium Effort).

Related to #3478

Approach

  1. Review current structure of strict_mode.go and validation_strict_mode.go (note: analysis shows there's already a validation_strict_mode.go file)
  2. Determine if consolidation into strict_mode_validation.go makes sense or if current split is intentional
  3. If consolidation is appropriate, merge validation logic into a single strict_mode_validation.go
  4. Move validateStrictMode and related methods to the consolidated file
  5. Update imports and references
  6. Verify all tests pass

Files to Modify

  • Review: pkg/workflow/strict_mode.go
  • Review: pkg/workflow/validation_strict_mode.go
  • Potentially create: pkg/workflow/strict_mode_validation.go
  • Update: Any files referencing these validation methods

Acceptance Criteria

  • Validation logic properly organized following codebase patterns
  • No duplication between strict_mode.go and validation files
  • All validation methods in appropriate validation file(s)
  • All imports correctly updated
  • All existing tests pass
  • Consistent naming with other *_validation.go files

Estimated Effort

1-2 hours

Benefits

  • Improved consistency with validation file naming patterns
  • Better organization of validation logic
  • Easier to locate strict mode validation code
  • Reduced confusion about where validation logic should live

Note

This task requires careful review of the existing validation_strict_mode.go file to determine the best consolidation approach.
Related to #3478

AI generated by Plan Command for #3478</issue_description>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

- Rename validation_strict_mode.go to strict_mode_validation.go for consistency
- Move validateStrictMode orchestrator from strict_mode.go to validation file
- Delete strict_mode.go as it's now redundant
- Update documentation references in validation.go, CONTRIBUTING.md, and specs/validation-architecture.md
- All tests pass successfully

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Extract validation methods from strict_mode.go Consolidate strict mode validation into strict_mode_validation.go Nov 8, 2025
Copilot AI requested a review from pelikhan November 8, 2025 17:20
@pelikhan pelikhan marked this pull request as ready for review November 8, 2025 18:19
Copilot AI review requested due to automatic review settings November 8, 2025 18:19
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR consolidates strict mode validation logic by merging two files (strict_mode.go and validation_strict_mode.go) into a single strict_mode_validation.go file. The changes include:

  • Moving the validateStrictMode() orchestrator function into strict_mode_validation.go
  • Deleting the now-redundant strict_mode.go file
  • Updating documentation references throughout to reflect the new file structure

However, the documentation also introduces references to pip_validation.go, npm_validation.go, and docker_validation.go which don't exist in the codebase.

Reviewed Changes

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

Show a summary per file
File Description
specs/validation-architecture.md Updates documentation to reference the consolidated strict_mode_validation.go file and mentions validation files with *_validation.go suffix
pkg/workflow/validation.go Updates file header comments to reference strict_mode_validation.go and introduces references to non-existent *_validation.go files
pkg/workflow/strict_mode_validation.go Adds the validateStrictMode() orchestrator function from deleted strict_mode.go with enhanced documentation
pkg/workflow/strict_mode.go File deleted - functionality moved to strict_mode_validation.go
CONTRIBUTING.md Updates validation architecture documentation with references to non-existent *_validation.go files

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

Comment on lines +9 to +11
// - pip_validation.go: Python package validation
// - npm_validation.go: NPM package validation
// - docker_validation.go: Docker image validation
Copy link

Copilot AI Nov 8, 2025

Choose a reason for hiding this comment

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

The files pip_validation.go, npm_validation.go, and docker_validation.go don't exist in the codebase. The actual files are pip.go, npm.go, and docker.go. These references should be corrected to match the actual filenames, or if a renaming is planned, those files should be renamed in the same PR.

Suggested change
// - pip_validation.go: Python package validation
// - npm_validation.go: NPM package validation
// - docker_validation.go: Docker image validation
// - pip.go: Python package validation
// - npm.go: NPM package validation
// - docker.go: Docker image validation

Copilot uses AI. Check for mistakes.
Comment on lines +152 to +154
- `pip_validation.go` - Python package validation
- `npm_validation.go` - NPM package validation
- `docker_validation.go` - Docker image validation
Copy link

Copilot AI Nov 8, 2025

Choose a reason for hiding this comment

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

The files pip_validation.go, npm_validation.go, and docker_validation.go don't exist in the codebase. The actual files containing validation logic are pip_validation.go (exists), npm_validation.go (exists), and docker_validation.go (exists) for validation functions, but the extraction and some validation is in pip.go, npm.go, and docker.go. This documentation should reference the actual existing files.

Suggested change
- `pip_validation.go` - Python package validation
- `npm_validation.go` - NPM package validation
- `docker_validation.go` - Docker image validation
- `pip.go` - Python package validation
- `npm.go` - NPM package validation
- `docker.go` - Docker image validation

Copilot uses AI. Check for mistakes.
**Architecture**: The strict mode validation is split across two files for better organization:
- `strict_mode.go` contains the main orchestrator that coordinates validation
- `validation_strict_mode.go` contains the individual validation function implementations
**Architecture**: All strict mode validation logic is consolidated in a single file following the `*_validation.go` naming pattern used throughout the codebase
Copy link

Copilot AI Nov 8, 2025

Choose a reason for hiding this comment

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

The claim that the *_validation.go naming pattern is "used throughout the codebase" is misleading. While pip_validation.go, npm_validation.go, and docker_validation.go exist, many other domain-specific files like pip.go, npm.go, docker.go, expression_safety.go, engine.go, mcp-config.go, and template.go don't follow this pattern. Consider revising to say "following the *_validation.go naming convention" without claiming it's used throughout.

Suggested change
**Architecture**: All strict mode validation logic is consolidated in a single file following the `*_validation.go` naming pattern used throughout the codebase
**Architecture**: All strict mode validation logic is consolidated in a single file following the `*_validation.go` naming convention

Copilot uses AI. Check for mistakes.
@pelikhan pelikhan merged commit 51e8bef into main Nov 8, 2025
76 checks passed
@pelikhan pelikhan deleted the copilot/extract-validation-methods-again branch November 8, 2025 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants