Refactor safe outputs subsystem to reduce duplication#9012
Merged
Conversation
Extract repetitive config generation patterns into reusable helper functions to reduce code duplication in safe_outputs_config_generation.go. Changes: - Created safe_outputs_config_generation_helpers.go with 9 helper functions - Reduced safe_outputs_config_generation.go from 991 to 854 lines (137 lines) - Extracted patterns for max config, allowed labels, target fields, etc. - All unit tests pass Impact: Reduced duplication, improved maintainability, easier to add new safe output types Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
Replace manual field checking with reflection-based implementation for HasSafeOutputsEnabled and GetEnabledSafeOutputToolNames to reduce code duplication and improve maintainability. Changes: - Created safe_outputs_config_helpers_reflection.go with reflection helpers - Reduced safe_outputs_config_helpers.go from 196 to 77 lines (119 line reduction) - Total: 166 lines vs original 196 (30 line reduction) - Maintained field-to-tool mapping in single location - All unit tests pass Benefits: - Adding new safe output types now requires only updating the mapping - Eliminated 150+ lines of manual field checking - Improved code maintainability Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Refactor pkg/workflow for semantic function clustering
Refactor safe outputs subsystem to reduce duplication
Jan 5, 2026
Contributor
|
@copilot format and lint go |
Contributor
Author
All Go files have been formatted and linted successfully. The code was already properly formatted - no changes were needed. Actions performed:
The code meets Go formatting standards. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The safe outputs subsystem had significant code duplication: 991 lines of repetitive if/then blocks in
safe_outputs_config_generation.goand 150+ lines of manual field checking in helper functions.Changes
Extracted config generation helpers (
safe_outputs_config_generation_helpers.go)safe_outputs_config_generation.gofrom 991 to 854 linesImplemented reflection-based field checking (
safe_outputs_config_helpers_reflection.go)safe_outputs_config_helpers.gofrom 196 to 77 linesExample
Before:
After:
Impact
Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
https://api.github.com/user/usr/bin/gh gh api user --jq .login fe-outputs ntmatter_extraction_security_test.go grep fe-outputs .go rgo/bin/grep grep -l fe-outputs est.go rgo/bin/grep fe-outputs mment_test.go ache/node/20.19./tmp/go-build1148239878/b001/_pkg_.a grep(http block)/usr/bin/gh gh api user --jq .login purge-campaign-lock-295535141 grep /usr/bin/infocmp l n_test.go ache/node/20.19.xterm-color infocmp -1 /ref/tags/v8 grep 742238718/gh-aw fe-outputs edule_preprocessapi grep 742238718/gh-aw(http block)/usr/bin/gh gh api user --jq .login /repos/actions/g/tmp/gh-aw-test-runs/20260105-161839-35855/test-orchestrator-1879498174/run-1003gh --jq(http block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
This section details on the original issue you should resolve
<issue_title>[refactor] Semantic Function Clustering Analysis: pkg/workflow/ Refactoring Opportunities</issue_title>
<issue_description>## Analysis Overview
This report analyzes the Go codebase in
pkg/workflow/for semantic function clustering and refactoring opportunities. The analysis examined 207 non-test files containing approximately 50,000+ lines of code.Files Analyzed
Sample of 30+ key files examined:
compiler.go,compiler_jobs.go,compiler_yaml.go,compiler_safe_outputs*.go*_validation.gofiles*_helpers.gofiles*_engine*.gofilesexpression_parser.go,expression_builder.go,expression_extraction.go,expression_validation.gosafe_outputs*.gofiles (9,437 total lines)create_*.gofiles, 6update_*.gofiles, 3close_*.gofilesKey Findings
1. EXCELLENT ORGANIZATION PATTERNS (Keep These!)
The codebase demonstrates strong organizational principles in several areas:
✅ Validation Files Architecture
The validation system is exceptionally well-organized:
Files:
✅ Expression Subsystem
Well-factored with clear separation of concerns:
expression_parser.go- Parsing logicexpression_builder.go- Builder pattern for constructionexpression_extraction.go- Extraction from markdownexpression_validation.go- Security validationexpression_nodes.go- Data structures✅ Helper File Conventions
Helper files follow consistent patterns with excellent documentation:
engine_helpers.go,config_helpers.go,update_entity_helpers.go,close_entity_helpers.go✅ Compiler Subsystem
Well-split into logical units:
compiler.go- Main orchestration (500 lines)compiler_jobs.go- Job generationcompiler_yaml.go- YAML generationcompiler_safe_outputs*.go- Safe outputs compilation (6 files)compiler_yaml_helpers.go- Shared YAML utilities2. HIGH-IMPACT REFACTORING OPPORTUNITIES
🔴 CRITICAL: Safe Outputs Configuration Explosion
Problem: The safe outputs subsystem has grown into 8 files with 9,437 lines and shows signs of duplication:
Issues Detected:
Configuration Parsing Duplication (
safe_outputs_config.go):parse*ConfigfunctionsConfiguration Generation Duplication (
safe_outputs_config_generation.go):💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.