-
Notifications
You must be signed in to change notification settings - Fork 43
Fix copilot assignment in create_issue by adding missing environment variable #12491
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: mnkiefer <[email protected]>
- Add missing 'assignees' field validation for create_issue type - Field accepts array of strings with max length 39 (GitHub username limit) - Enables workflows like security-alert-burndown to use assignees: copilot - All 142 workflows recompile successfully - Tests pass for handler config including assignees Co-authored-by: mnkiefer <[email protected]>
|
@copilot This was already possible before |
You're absolutely right! The My change only adds validation for the field (ensuring it's an array of strings with max 39 chars each), but doesn't enable any new functionality. The feature was working before, just without explicit validation in the schema. Should I close this PR since it's not actually fixing a bug, just adding validation? Or would you prefer to keep it for the added safety of validating assignee format? |
|
@copilot No, but check my the json snippet I provided would cause a validation error and skip the agent assignment step |
I'm looking at the JSON snippet from the problem statement - it's a The Could you clarify what specific validation error you're seeing? Is it:
I want to make sure I fix the actual issue you're encountering. |
|
@copilot assignment is skipped with the json smippet the agent provided. |
I've traced through the code and the JSON snippet should work correctly:
My validation change adds the Could you share:
This will help me understand what's actually going wrong so I can fix the right issue. |
|
@copilot No error, but the step was skipped. |
I found the root cause! The issue is NOT related to my validation change. There's a bug in the compiler where The compiler checks if "copilot" is in assignees (line 179 in create_issue.go) and should set the env var, but This means copilot assignment has been broken since the unified safe outputs handler was introduced, regardless of my validation changes. My validation PR is unrelated to this issue. Should I:
|
|
@copilot fix the underlying issue |
- Add GH_AW_ASSIGN_COPILOT environment variable in addAllSafeOutputConfigEnvVars - Variable is set to "true" when copilot is in create-issue assignees - Enables copilot assignment to work in unified safe outputs handler - Fixes issue where assignment step was skipped despite assignees: copilot config - Recompiled workflows: security-alert-burndown, duplicate-code-detector, breaking-change-checker Co-authored-by: mnkiefer <[email protected]>
- Add comprehensive tests for copilot assignment env var - Verify env var is set when copilot is in assignees - Verify env var is not set when copilot is absent - Test mixed assignees scenario (copilot + other users) Co-authored-by: mnkiefer <[email protected]>
Fixed in commits 41c135a and 95804da. The issue was that Now when workflows have env:
GH_AW_ASSIGN_COPILOT: "true"This enables the |
- Test nil assignees (env var not set) - Test empty assignees array (env var not set) - Ensures hasCopilotAssignee handles edge cases correctly - Addresses code review feedback Co-authored-by: mnkiefer <[email protected]>
|
@copilot Only keep the fix, discard the extra validation you added, and recompile workflows |
- Revert assignees field validation from safe_output_validation_config.go - Keep the GH_AW_ASSIGN_COPILOT environment variable fix in compiler_safe_outputs_env.go - Keep comprehensive tests for the env var fix - Recompile all 142 workflows without the validation schema changes - Per feedback: validation was not needed, only the env var fix Co-authored-by: mnkiefer <[email protected]>
Done in commit e48d3d1. Reverted the validation schema changes from
The workflows now have |
The
GH_AW_ASSIGN_COPILOTenvironment variable wasn't being set in the unified safe outputs handler, causing copilot assignment to be skipped.Root Cause
When workflows used
assignees: copilotin theirsafe-outputs.create-issueconfiguration, the copilot assignment step was being skipped because theGH_AW_ASSIGN_COPILOTenvironment variable wasn't set. Thecreate_issue.cjshandler checks for this variable to determine whether to queue issues for copilot assignment.Changes
Fixed copilot assignment environment variable (
pkg/workflow/compiler_safe_outputs_env.go)Added logic to set
GH_AW_ASSIGN_COPILOT: "true"when copilot is in assignees:Added comprehensive tests (
pkg/workflow/compiler_safe_outputs_env_copilot_test.go)Impact
Workflows that use
assignees: copilotin theirsafe-outputs.create-issueconfiguration now correctly queue created issues for copilot assignment:The compiled workflows now include:
Workflows using assignees (security-alert-burndown, duplicate-code-detector, breaking-change-checker) now have the environment variable set correctly and copilot assignment works as intended.
Testing
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.