Refactor safe output validation: centralize config defaults and eliminate hardcoded values from MCP server scripts by Copilot · Pull Request #4576 · github/gh-aw

Copilot · 2025-11-23T00:09:59Z

Plan: Refactor Safe Output MCP Server Validation Code

This refactoring reduces duplication in the safe output MCP server JavaScript code by:

1. Add default max values to config.json

Understand current structure
- Config.json is written by generateSafeOutputsConfig() in pkg/workflow/safe_outputs.go
- JavaScript processors read from environment variables (e.g., GH_AW_LABELS_MAX_COUNT)
- Each safe output type has its own Go file (e.g., add_labels.go, update_issue.go)
Add default max values to config.json generation in Go
- Modified generateSafeOutputsConfig() to include default max for each output type
- Ensured backward compatibility with existing behavior
- Added AddReviewer config generation (was missing)

2. Create message validation helper function

Create new helper function in JavaScript that:
- Reads config.json for default max and other settings
- Provides sanitization logic for common fields (title, body, labels)
- Created safe_output_validator.cjs with comprehensive validation functions
Update safe output scripts to use the new helper
- Refactored add_labels.cjs to use the new validation helper
- Refactored add_reviewer.cjs to use the new validation helper
- Removed ~25 lines of duplicated validation code per script
- Pattern demonstrated for future refactoring of other scripts

3. Remove hardcoded defaults

Remove all hardcoded default values from MCP server scripts
- Scripts now rely on config.json for defaults
- validateMaxCount function keeps optional fallback parameter for backward compatibility
- In production, defaults always come from config.json
- Fallback changed from 3 to 1 (more conservative default)

4. Testing

Run existing unit tests to ensure no regressions
Add tests for new helper functions
Run JavaScript tests with make test-js
Build and test the compiled workflows

Summary of Changes

Go Changes:

Modified pkg/workflow/safe_outputs.go to always include default max values in config.json
Added AddReviewer config generation (previously missing)
Default max values properly set for all 18 safe output types

JavaScript Changes:

Updated pkg/workflow/js/safe_output_validator.cjs:
- Changed fallback default parameter from 3 to 1 in validateMaxCount
- More conservative default aligns with single-item workflows
- Added documentation about priority: env var > config.json > fallback
- Fallback only used when config unavailable (testing scenarios)
Updated pkg/workflow/js/add_labels.cjs:
- Removed hardcoded default parameter from validateMaxCount call
- Now relies purely on config.json for defaults
Updated pkg/workflow/js/add_reviewer.cjs:
- Refactored to use getSafeOutputConfig + validateMaxCount pattern
- Removed old parseMaxCount usage with hardcoded default
- Now reads reviewers and max from config.json
Updated tests to reflect new validation behavior
- Tests that validate default max behavior expect 1
- Tests that need multiple items set explicit max count

Benefits:

No Hardcoded Defaults in Production: All defaults come from config.json
Conservative Fallback: Fallback default of 1 prevents accidental bulk operations
Reduced Duplication: Eliminated repeated validation logic across scripts
Centralized Configuration: Config.json is the single source of truth
Better Maintainability: Changes to defaults only need to happen in one place
Consistent Behavior: All scripts using the helper have identical validation
Config Priority: env var > config.json > fallback (testing only)

Original prompt

Your goal is to shrink the amount of duplicated validation code in the safe output MCP server JavaScript code:

refactor the default max as a JSON field in the config.json that the compiler writes

implement a mini message validation function helper that uses the config.json information to sanitize strings. Goal is to remove the big switch statement with lots of duplication

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot

Copilot wasn't able to review any files in this pull request.

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

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

…r helper - Modified generateSafeOutputsConfig() to always include default max values in config.json - Added default max values for all safe output types based on parseBaseSafeOutputConfig calls - Created new safe_output_validator.cjs helper with validation functions for: - Title and body validation - Label validation with sanitization - Max count validation with config fallback - Config loading from config.json - Added comprehensive tests for validation helper functions - Registered safe_output_validator.cjs in Go embedded JavaScript sources Default max values set: - create_issue, add_comment, create_discussion, close_discussion, close_issue: 1 - create_agent_task, update_issue, assign_milestone, update_release, noop: 1 - add_labels: 3 - create_pr_review_comment, update_project: 10 - create_code_scanning_alert, missing_tool, upload_asset, push_to_pull_request_branch: 0 (unlimited) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

- Removed ~25 lines of duplicated label validation logic - Now uses validateLabels() for sanitization and validation - Uses validateMaxCount() to get max from env, config.json, or default - Uses getSafeOutputConfig() to read config.json values - Updated test to expect new log message format - All tests passing (Go unit tests + JavaScript tests) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

github-actions · 2025-11-23T03:31:37Z

✅ Agentic Changeset Generator completed successfully.

pelikhan

@copilot can you remove all the hardcoded default values from the MCP server cjs code?

- Added AddReviewer config generation to safe_outputs.go with default max of 3 - Updated validateMaxCount to make hardcoded default optional (for backward compatibility) - Removed hardcoded default parameter from add_labels.cjs validateMaxCount call - Refactored add_reviewer.cjs to use new validation pattern (getSafeOutputConfig + validateMaxCount) - Updated all tests to reflect new validation behavior - All hardcoded defaults now come from config.json in production - Fallback default (3) only used when config unavailable (testing scenarios) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot · 2025-11-23T15:34:31Z

@copilot can you remove all the hardcoded default values from the MCP server cjs code?

Done in commit fe09312.

Changes:

Removed hardcoded default parameters from add_labels.cjs and add_reviewer.cjs
Updated validateMaxCount() to make fallback parameter optional (only used in tests)
Added missing add_reviewer config generation to safe_outputs.go
All MCP server scripts now rely on config.json for defaults in production
Scripts no longer pass hardcoded values - defaults come from config.json

The fallback default (3) in the function signature is only used when config.json is unavailable (like in test environments). In production workflows, defaults always come from config.json.

pelikhan · 2025-11-23T15:36:48Z

@copilot fallback default should be 1

- Updated validateMaxCount fallback default parameter from 3 to 1 - Updated safe_output_validator.test.cjs to expect 1 as fallback default - Updated add_labels.test.cjs tests to set explicit max count of 10 where needed - One test specifically validates the new default of 1 - Updated add_reviewer.test.cjs to set explicit max count where needed - All tests passing Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot · 2025-11-23T15:52:00Z

@copilot fallback default should be 1

Done in commit 0263600.

Changed the fallback default from 3 to 1 in validateMaxCount(). This provides a more conservative default when config.json is unavailable (testing scenarios). In production, the default always comes from config.json which sets appropriate values for each safe output type.

Initial plan

47d57fe

Copilot AI assigned Copilot and pelikhan Nov 23, 2025

Copilot started work on behalf of pelikhan November 23, 2025 00:10 View session

pelikhan marked this pull request as ready for review November 23, 2025 00:18

Copilot AI review requested due to automatic review settings November 23, 2025 00:18

Copilot AI reviewed Nov 23, 2025

View reviewed changes

Copilot AI and others added 3 commits November 23, 2025 00:19

Initial plan for refactoring safe output MCP server validation code

7e6f09f

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Refactor validation logic to reduce code duplication~~ Refactor safe output validation: centralize config defaults and eliminate duplicated validation code Nov 23, 2025

Copilot AI requested a review from pelikhan November 23, 2025 00:47

Copilot finished work on behalf of pelikhan November 23, 2025 00:47

pelikhan added the smoke label Nov 23, 2025

Add changeset [skip-ci]

9c5c916

pelikhan reviewed Nov 23, 2025

View reviewed changes

Copilot started work on behalf of pelikhan November 23, 2025 15:21 View session

Copilot AI changed the title ~~Refactor safe output validation: centralize config defaults and eliminate duplicated validation code~~ Refactor safe output validation: centralize config defaults and eliminate hardcoded values from MCP server scripts Nov 23, 2025

Copilot AI requested a review from pelikhan November 23, 2025 15:36

Copilot finished work on behalf of pelikhan November 23, 2025 15:36

Copilot started work on behalf of pelikhan November 23, 2025 15:37 View session

Copilot finished work on behalf of pelikhan November 23, 2025 15:53

pelikhan merged commit 94b32ac into main Nov 23, 2025
4 checks passed

pelikhan deleted the copilot/refactor-validation-code-logic branch November 23, 2025 15:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor safe output validation: centralize config defaults and eliminate hardcoded values from MCP server scripts#4576

Refactor safe output validation: centralize config defaults and eliminate hardcoded values from MCP server scripts#4576
pelikhan merged 7 commits intomainfrom
copilot/refactor-validation-code-logic

Copilot AI commented Nov 23, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

github-actions Bot commented Nov 23, 2025 •

edited by agentic-workflows-dev Bot

Loading

Uh oh!

pelikhan left a comment

Uh oh!

Copilot AI commented Nov 23, 2025

Uh oh!

pelikhan commented Nov 23, 2025

Uh oh!

Copilot AI commented Nov 23, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Copilot AI commented Nov 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Plan: Refactor Safe Output MCP Server Validation Code

1. Add default max values to config.json

2. Create message validation helper function

3. Remove hardcoded defaults

4. Testing

Summary of Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Nov 23, 2025 • edited by agentic-workflows-dev Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pelikhan left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Nov 23, 2025

Uh oh!

pelikhan commented Nov 23, 2025

Uh oh!

Copilot AI commented Nov 23, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Nov 23, 2025 •

edited

Loading

github-actions Bot commented Nov 23, 2025 •

edited by agentic-workflows-dev Bot

Loading