-
Notifications
You must be signed in to change notification settings - Fork 35
Migrate safe output handlers to config object pattern and eliminate handler-specific env vars #8019
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: pelikhan <[email protected]>
Co-authored-by: pelikhan <[email protected]>
Co-authored-by: pelikhan <[email protected]>
| * @param {Array<Object>} messages - All messages from agent output | ||
| * @returns {Map<string, Array<Object>>} Messages grouped by type | ||
| */ | ||
| function groupMessagesByType(messages) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot do not group message, process them in order of appearance in the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to process messages sequentially in order of appearance. Removed grouping logic and processing order enforcement. Messages are now handled one-by-one as they appear in the agent output file. (commit 4b7b51e)
Co-authored-by: pelikhan <[email protected]>
|
@copilot update compiler and recompile workflows |
Modified buildConsolidatedSafeOutputsJob to use handler manager for: - create_issue - add_comment - create_discussion - close_issue - close_discussion These handlers are now dispatched through a single "Process Safe Outputs" step instead of individual steps, simplifying workflow files. Co-authored-by: pelikhan <[email protected]>
Updated compiler to use handler manager for create_issue, add_comment, create_discussion, close_issue, and close_discussion. Recompiled all workflows - they now use a single "Process Safe Outputs" step instead of individual steps for these types. (commit d54e800) |
|
🎉 Yo ho ho! Smoke Copilot Safe Inputs found the treasure and completed successfully! ⚓💰 |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
✅ Firewall validation complete... Smoke Codex Firewall confirmed network sandboxing is operational. 🛡️ |
|
🎉 Yo ho ho! Changeset Generator found the treasure and completed successfully! ⚓💰 |
|
🤖 DIAGNOSTIC COMPLETE: Smoke Copilot No Firewall STATUS: ALL_UNITS_OPERATIONAL. MISSION_SUCCESS. |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
📰 VERDICT: Smoke Copilot Playwright has concluded. All systems operational. This is a developing story. 🎤 |
This comment has been minimized.
This comment has been minimized.
close_discussion.cjs is already fully migrated to use the config object. All handler-specific configuration (required_labels, required_title_prefix, required_category, target, max) is read from the config parameter. Only approved environment variables remain:
No changes needed. |
|
@copilot review each safe output JavaScript handler file and ensure it uses the config |
All 8 safe output handlers correctly use the config object pattern. Here's the verification: ✅ create_issue.cjs - Reads ✅ add_comment.cjs - Reads ✅ add_labels.cjs - Passes config to ✅ close_issue.cjs - Passes config to ✅ close_discussion.cjs - Reads ✅ create_discussion.cjs - Reads ✅ update_issue.cjs - Uses factory pattern via ✅ update_discussion.cjs - Uses factory pattern, reads ✅ update_runner.cjs - Reads Environment variables check: No handler-specific env vars found. Only approved variables remain:
All handlers successfully migrated to config object pattern. Environment reduced from 30+ to 3 variables. |
|
📰 VERDICT: Smoke Copilot Playwright has concluded. All systems operational. This is a developing story. 🎤 |
|
🎉 Yo ho ho! Smoke Copilot Safe Inputs found the treasure and completed successfully! ⚓💰 |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
✅ Firewall validation complete... Smoke Codex Firewall confirmed network sandboxing is operational. 🛡️ |
|
🎉 Yo ho ho! Changeset Generator found the treasure and completed successfully! ⚓💰 |
|
🤖 ALERT: Smoke Copilot No Firewall failed. ANOMALY_DETECTED. REPAIR_REQUIRED. |
Smoke Test ResultsLast 2 Merged PRs:
Test Results: Overall: PASS cc: @pelikhan (author)
|
Smoke Test Results✅ File Writing: Created Status: PARTIAL PASS - 2/3 tests passed (gh CLI requires workflow context)
|
|
Merged PRs:
|
Smoke Test Results (Claude Sonnet 4.5)Recent PRs:
Tests:
Status: PASS (5/6 tests passed)
|
|
Recent merged PRs: Add description to applyTo field in included_file_schema.json; Fix Go and JavaScript linting errors
|
Smoke Test Results❌ Playwright MCP: Inspector started but couldn't execute navigation test Overall Status: FAIL (1/3 tests passed)
|
|
@copilot remove the github-token field for the safe output handler that are refactored
|
Removed the github-token field from JSON schema for the 8 refactored safe output handlers that now use the centralized handler manager with config object pattern: **Schema Changes (main_workflow_schema.json):** - ✅ Removed `github-token` from `create-issue` - ✅ Removed `github-token` from `add-comment` - ✅ Removed `github-token` from `create-discussion` - ✅ Removed `github-token` from `close-issue` - ✅ Removed `github-token` from `close-discussion` - ✅ Removed `github-token` from `update-issue` - ✅ Removed `github-token` from `update-discussion` - ⏭️ `add-labels` didn't have github-token field (no change needed) **Rationale:** These handlers now use the centralized handler manager pattern where configuration is passed via a JSON config object (GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG). The handlers no longer support per-handler token overrides - they use the global workflow token or safe-outputs level token. **Go Struct (compiler_types.go):** The `BaseSafeOutputConfig.GitHubToken` field remains in the Go struct for backward compatibility with non-refactored handlers (create-pull-request, update-project, etc.) that still use individual tokens. **Verification:** - Rebuilt binary successfully - Compiled duplicate-code-detector.md workflow successfully - Verified github-token field removed from all 7 applicable handlers Addresses comment to remove github-token from refactored handlers. Co-authored-by: pelikhan <[email protected]>
Removed github-token field from JSON schema for all 7 refactored handlers (create-issue, add-comment, create-discussion, close-issue, close-discussion, update-issue, update-discussion). The add-labels handler didn't have this field. The Go struct |
Safe Output Handler Config Object Migration - Complete
This PR migrates all safe output handlers from using individual environment variables to a centralized config object pattern, reducing environment variables from 30+ to just 3.
Status: ✅ COMPLETE
All 8 handlers fully migrated to config object pattern
Handler Status (8/8 complete):
Remaining Work:
Changes Summary
Configuration Updates - 8 Handlers (ALL COMPLETE):
add_labels.cjs: Config passed via processSafeOutput
max,allowed,targetclose_issue.cjs + close_entity_helpers.cjs:
max,target,required_labels,required_title_prefixclose_discussion.cjs:
required_labels,required_title_prefix,required_category,targetmax,target,required_labels,required_title_prefix,required_categorycreate_discussion.cjs (FULLY UPDATED):
category,allowed_repos,title_prefix,labels,close_older_discussions,expiresfrom configmax,category,title_prefix,labels,allowed_labels,allowed_repos,close_older_discussions,expiresadd_comment.cjs:
hide_older_commentsandtargetfrom config objectmax,target,hide_older_commentscreate_issue.cjs:
labels,title_prefix,expires,allowed_labels,allowed_reposfrom configmax,labels,title_prefix,expires,allowed_labels,allowed_reposupdate_issue.cjs:
allow_status,allow_title,allow_body,targetfrom configmax,target,allow_status,allow_title,allow_bodyupdate_discussion.cjs:
allow_title,allow_body,allow_labels,allowed_labels,targetfrom configmax,target,allow_title,allow_body,allow_labels,allowed_labelsSchema Cleanup
Removed
github-tokenfield from refactored handlers:The
github-tokenfield has been removed from the JSON schema for all 8 refactored safe output handlers. These handlers now use the centralized handler manager pattern where configuration is passed via theGH_AW_SAFE_OUTPUTS_HANDLER_CONFIGJSON object. The handlers no longer support per-handler token overrides - they use the global workflow token or safe-outputs level token.Removed from:
create-issueschemaadd-commentschemacreate-discussionschemaclose-issueschemaclose-discussionschemaupdate-issueschemaupdate-discussionschemaadd-labelsdidn't have github-token field (no change needed)Note: The
BaseSafeOutputConfig.GitHubTokenfield remains in the Go struct (compiler_types.go) for backward compatibility with non-refactored handlers (create-pull-request, update-project, etc.) that still use individual tokens.Env Var Cleanup (100% COMPLETE ✅)
ALL handler-specific environment variables eliminated:
GH_AW_ISSUE_LABELS→ config.labelsGH_AW_ISSUE_TITLE_PREFIX→ config.title_prefixGH_AW_ISSUE_EXPIRES→ config.expiresGH_AW_ISSUE_ALLOWED_LABELS→ config.allowed_labelsGH_AW_ALLOWED_REPOS→ config.allowed_reposGH_AW_HIDE_OLDER_COMMENTS→ config.hide_older_commentsGH_AW_COMMENT_TARGET→ config.targetGITHUB_AW_COMMENT_DISCUSSION→ (removed, not needed)GH_AW_LABELS_ALLOWED→ config.allowedGH_AW_LABELS_MAX_COUNT→ config.maxGH_AW_LABELS_TARGET→ config.targetGH_AW_UPDATE_TARGET→ config.targetGH_AW_UPDATE_TITLE→ config.allow_titleGH_AW_UPDATE_BODY→ config.allow_bodyGH_AW_UPDATE_LABELS→ config.allow_labelsGH_AW_CLOSE_OLDER_DISCUSSIONS→ config.close_older_discussionsGH_AW_DISCUSSION_TITLE_PREFIX→ config.title_prefixGH_AW_DISCUSSION_LABELS→ config.labelsGH_AW_DISCUSSION_EXPIRES→ config.expiresRemaining (3 env vars only):
GH_AW_SAFE_OUTPUTS_STAGED- Global staged mode flagGH_AW_TARGET_REPO_SLUG- Target repository override (optional)GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG- Handler configuration JSONAchievement: Reduced from 30+ env vars to 3 env vars! 🎉
Implementation Complete
Phase 1-5: ✅ COMPLETE
Phase 6: Tests & Validation (🔄 Pending)
Phase 7: Compilation (⏳ Pending)
Benefits Achieved
Files Affected
JavaScript (ALL COMPLETE):
Go (COMPLETE):
Schema (COMPLETE):
Workflows:
Next Steps
Run full test suite and recompile all 126 workflows to complete the migration.
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.