-
Notifications
You must be signed in to change notification settings - Fork 34
Set common environment variables at job level in consolidated safe_outputs job #7049
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 <4175913+pelikhan@users.noreply.github.com>
|
@copilot review other safe output environment variable and set common ones at job level. |
- Added job-level Env map with GH_AW_WORKFLOW_ID in buildConsolidatedSafeOutputsJob - Removed duplicate GH_AW_WORKFLOW_ID from create_pull_request step config (now inherited from job level) - Updated integration tests to check job.Env for GH_AW_WORKFLOW_ID in consolidated jobs - Tests now properly verify job-level env vars instead of looking for them in step content Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- Updated create_pull_request_env_vars_test.go to not expect GH_AW_WORKFLOW_ID in step-level env vars - Added comments explaining that GH_AW_WORKFLOW_ID is now at job level - Recompiled all workflow lock files with new job-level env configuration - All tests pass (except pre-existing TestCheckFirewallDisable failure) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot use the workflow id, not the JOB id. It should match the workflow file name without extension |
…to job level - Changed GH_AW_WORKFLOW_ID from mainJobName to workflow filename (without extension) - Moved common environment variables to job level: GH_AW_WORKFLOW_NAME, GH_AW_WORKFLOW_SOURCE, GH_AW_TRACKER_ID, GH_AW_ENGINE_ID, GH_AW_ENGINE_VERSION, GH_AW_ENGINE_MODEL, GH_AW_SAFE_OUTPUTS_STAGED, GH_AW_TARGET_REPO_SLUG, GH_AW_SAFE_OUTPUT_MESSAGES - Added buildJobLevelSafeOutputEnvVars() to build job-level environment variables - Added buildStepLevelSafeOutputEnvVars() to build step-level env vars (excluding job-level ones) - Updated all consolidated step configs to use buildStepLevelSafeOutputEnvVars() - Individual job builders still use buildStandardSafeOutputEnvVars() for backward compatibility Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- Workflow lock files now have common env vars at job level instead of duplicated in steps - GH_AW_WORKFLOW_ID now uses workflow filename instead of job name - 118/123 workflows compiled successfully (5 pre-existing failures) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed in commit fb7c96f. Changed GH_AW_WORKFLOW_ID from mainJobName (e.g., "agent") to the workflow filename without extension (e.g., "ai-moderator"). The workflow filename is extracted using
Done in commit fb7c96f. Moved the following common environment variables to job level:
This reduces YAML file sizes (~6-7KB per workflow) and improves maintainability. |
|
@copilot use helper function to convert workflow file name to workflow id |
- Created GetWorkflowIDFromPath() helper function in compiler_yaml_helpers.go - Updated compiler_safe_outputs_consolidated.go to use the helper function - Removed unused filepath and strings imports from compiler_safe_outputs_consolidated.go - Added comprehensive test coverage for GetWorkflowIDFromPath() - Renamed workflowFilename parameter to workflowID for clarity in buildJobLevelSafeOutputEnvVars() Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in commit dd43500. Created |
|
@copilot merge main |
…kflow-id # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
Plan: Set GH_AW_WORKFLOW_ID at Job Level in Safe Outputs Job ✅
Problem
Integration tests were failing because the
GH_AW_WORKFLOW_IDenvironment variable was missing from most safe output job configurations. Additionally, common environment variables were being duplicated in every step, leading to bloated YAML files.Root Cause
buildConsolidatedSafeOutputsJobfunction wasn't settingGH_AW_WORKFLOW_IDat the job levelGH_AW_WORKFLOW_IDwas incorrectly using the job name instead of the workflow filenameSolution
GH_AW_WORKFLOW_IDat the job level using the workflow filename (not the job name)Changes Completed ✅
GetWorkflowIDFromPath()helper function for extracting workflow ID from markdown pathsbuildConsolidatedSafeOutputsJobto use the helper functionworkflowFilenameparameter toworkflowIDfor claritybuildJobLevelSafeOutputEnvVars()to build common env vars at job levelbuildStepLevelSafeOutputEnvVars()for step-specific env vars (excludes job-level vars)buildStepLevelSafeOutputEnvVars()GH_AW_WORKFLOW_IDto use workflow filename instead of job nameEnvironment Variables at Job Level
The following variables are now set once at the job level instead of being duplicated in each step:
GH_AW_WORKFLOW_ID- workflow filename (without extension), e.g., "ai-moderator"GH_AW_WORKFLOW_NAME- workflow name from frontmatter, e.g., "AI Moderator"GH_AW_WORKFLOW_SOURCE- workflow source (if configured)GH_AW_WORKFLOW_SOURCE_URL- source URL (if source configured)GH_AW_TRACKER_ID- tracker ID (if configured)GH_AW_ENGINE_ID- engine ID, e.g., "copilot"GH_AW_ENGINE_VERSION- engine version (if configured)GH_AW_ENGINE_MODEL- engine model, e.g., "gpt-5-mini" (if configured)GH_AW_SAFE_OUTPUTS_STAGED- staged mode flag (if applicable)GH_AW_TARGET_REPO_SLUG- target repo for trial mode (if applicable)GH_AW_SAFE_OUTPUT_MESSAGES- messages config (if configured)Benefits
✅ Reduced YAML file sizes (~6-7KB reduction per workflow on average)
✅ Improved maintainability - common variables defined once at job level
✅ Fixed
GH_AW_WORKFLOW_IDto use correct workflow identifier✅ Created reusable helper function to reduce code duplication
✅ Integration tests now pass
✅ Backward compatibility maintained for individual job builders
✅ Up to date with main branch
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.