Conversation
There was a problem hiding this comment.
Pull request overview
This PR removes explicit validation for git tool requirements in safe-outputs workflows, with the intention of simplifying the compilation process by relying on automatic git command injection. However, the automatic injection does not handle all cases that the removed validation covered.
Changes:
- Removed
validateGitToolForSafeOutputsfunction that validated git tool availability for safe-outputs features - Removed corresponding unit tests (TestValidateGitToolForSafeOutputs)
- Updated integration tests to expect successful compilation for previously invalid configurations
- Removed the validation call from the workflow compilation pipeline
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/workflow/tools_validation.go | Removed validateGitToolForSafeOutputs function and replaced it with a comment explaining the removal |
| pkg/workflow/tools_validation_test.go | Removed TestValidateGitToolForSafeOutputs test suite and added explanatory comment; removed unused strings import |
| pkg/workflow/git_tool_validation_integration_test.go | Updated integration tests to expect successful compilation for bash: false and bash: [echo] cases that previously failed validation |
| pkg/workflow/compiler_orchestrator_workflow.go | Removed the call to validateGitToolForSafeOutputs from ParseWorkflowFile and added comment about automatic git command injection |
Comments suppressed due to low confidence (1)
pkg/workflow/git_tool_validation_integration_test.go:74
- The comment states "git commands auto-added" but this is incorrect for the bash: false case. When bash is explicitly set to false, the compiler's git injection logic does not override it. The git commands are NOT auto-added in this scenario - bash remains false and gets deleted from tools entirely.
This test name and comment are misleading. The actual behavior depends on the initial bash configuration:
- bash: ["echo"] → git commands are merged in (this works)
- bash: false → git commands are NOT added (silently broken)
The test should verify the actual compiled output to ensure git commands are present in the final workflow.
name: "create-pull-request with bash: [echo] - OK (git commands auto-added)",
workflow: `---
name: Test Create PR With Limited Bash
engine: copilot
on: workflow_dispatch
tools:
bash: ["echo", "ls"]
safe-outputs:
create-pull-request:
title-prefix: "[auto] "
---
Test workflow that uses create-pull-request without git in allowed commands.
The compiler will automatically add git commands to the bash allowlist.
`,
expectError: false, // Git commands are auto-injected
},
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Details
The changes remove the manual validation of git tool requirements for safe outputs workflows. Instead of manually checking git tool configuration, the compiler will now automatically inject necessary git commands when safe outputs features like create-pull-request or push-to-pull-request-branch are used.
This simplifies the workflow compilation process and makes it more flexible, allowing the compiler to handle git command injection automatically.