-
Notifications
You must be signed in to change notification settings - Fork 32
Remove AWF cleanup steps from workflow generation #3297
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
|
@copilot and the "Cleanup awf resources" step |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
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.
Pull Request Overview
This PR removes AWF (agentic workflow firewall) cleanup step functionality from the Copilot engine. The cleanup steps were previously executed both before (pre-execution) and after (post-execution) the workflow, but are no longer needed.
Key changes:
- Removed cleanup step generation functions (
generateAWFCleanupStepandgenerateAWFPostExecutionCleanupStep) - Removed cleanup step invocations from installation and cleanup phases
- Removed associated tests for cleanup script workspace path handling
- Updated all compiled workflow lock files to reflect the removal of cleanup steps
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
pkg/workflow/copilot_engine.go |
Removed cleanup step generation functions and their invocations from installation and cleanup methods |
pkg/workflow/firewall_args_test.go |
Removed tests for cleanup script workspace path validation |
pkg/workflow/firewall_args_integration_test.go |
Removed integration tests for cleanup script workspace path handling |
.github/workflows/*.lock.yml |
Updated compiled workflow files to remove cleanup steps from installation and post-execution phases |
Comments suppressed due to low confidence (1)
pkg/workflow/copilot_engine.go:1
- [nitpick] The
awfVersionvariable can be simplified by extracting it inline. Consider refactoring toawfInstall := generateAWFInstallationStep(getVersionOrEmpty(firewallConfig))or extracting the version directly in the function call for better readability.
package workflow
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Plan: Remove AWF Cleanup Steps
generateAWFCleanupStep)generateAWFPostExecutionCleanupStep)GetInstallationStepsandGetCleanupStepfirewall_args_test.goChanges Made
Code Changes
generateAWFCleanupStepfunction - Deleted the function that generated the pre-execution "Cleanup any existing awf resources" stepgenerateAWFPostExecutionCleanupStepfunction - Deleted the function that generated the post-execution "Cleanup awf resources" stepGetInstallationSteps- Removed the call togenerateAWFCleanupStepand the unusedcleanupScriptvariableGetCleanupStep- Simplified to always return an empty step with a comment explaining that cleanup steps have been removedTest Changes
TestCleanupScriptWorkspacePath- Deleted the entire test function fromfirewall_args_test.goas it tested functionality that no longer existsTestCleanupScriptWorkspaceIntegration- Deleted the integration test fromfirewall_args_integration_test.gothat verified cleanup steps in compiled workflowsWorkflow Lock Files
All workflow lock files have been recompiled and the cleanup steps have been removed:
.github/workflows/tests/test-firewall-default.lock.yml)Verification
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.