Make centralized slash-command workflows emit non-required workflow_dispatch inputs#32480
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot the compiler should raise an error on required: true and slash_command/label_command |
…gers Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot recompile and fix agentic workflows with that error |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds compiler validation intended to address workflow_dispatch.inputs.*.required usage in workflows that use slash_command / label_command, along with new tests covering the resulting behavior.
Changes:
- Added a new compiler validation step that rejects
workflow_dispatchinputs markedrequired: truewhenslash_commandorlabel_commandtriggers are present. - Added compile tests asserting compilation fails when required
workflow_dispatchinputs are declared alongside centralizedslash_commandorlabel_command. - Introduced a new validation helper (
validateCommandWorkflowDispatchInputs) to scan raw frontmatter for required dispatch inputs.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/compiler_validators.go | Invokes new validation step during compilation. |
| pkg/workflow/command_dispatch_validation.go | Implements validation that errors on required: true workflow_dispatch inputs for command-trigger workflows. |
| pkg/workflow/slash_command_centralized_compile_test.go | Adds regression tests that expect compilation to fail when required dispatch inputs are present. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 3
| // Validate workflow_dispatch required inputs with slash/label command triggers. | ||
| log.Printf("Validating workflow_dispatch input requirements for command triggers") | ||
| if err := validateCommandWorkflowDispatchInputs(workflowData); err != nil { | ||
| return formatCompilerError(markdownPath, "error", err.Error(), err) | ||
| } |
| func TestSlashCommandRejectsRequiredDispatchInputs(t *testing.T) { | ||
| tmpDir := testutil.TempDir(t, "workflow-centralized-slash-dispatch-inputs-test") | ||
|
|
||
| markdownPath := filepath.Join(tmpDir, "scout.md") | ||
| content := `--- | ||
| on: | ||
| slash_command: | ||
| name: scout | ||
| strategy: centralized | ||
| workflow_dispatch: | ||
| inputs: | ||
| topic: | ||
| description: "Research topic" | ||
| required: true | ||
| type: string | ||
| tools: | ||
| github: | ||
| allowed: [list_issues] | ||
| --- | ||
|
|
||
| # Scout | ||
| ` | ||
| require.NoError(t, os.WriteFile(markdownPath, []byte(content), 0644)) | ||
|
|
||
| compiler := NewCompiler() | ||
| err := compiler.CompileWorkflow(markdownPath) | ||
| require.Error(t, err) | ||
| require.Contains(t, err.Error(), "on.workflow_dispatch.inputs.topic.required: true is not allowed when using slash_command") | ||
|
|
| // validateCommandWorkflowDispatchInputs rejects required workflow_dispatch inputs when | ||
| // slash_command or label_command triggers are configured. | ||
| // Returns an error if any workflow_dispatch input has required: true. | ||
| func validateCommandWorkflowDispatchInputs(workflowData *WorkflowData) error { | ||
| if workflowData == nil || workflowData.RawFrontmatter == nil { | ||
| return nil | ||
| } | ||
|
|
||
| hasSlashCommand := len(workflowData.Command) > 0 | ||
| hasLabelCommand := len(workflowData.LabelCommand) > 0 | ||
| if !hasSlashCommand && !hasLabelCommand { | ||
| return nil | ||
| } | ||
|
|
||
| onMap, ok := workflowData.RawFrontmatter["on"].(map[string]any) | ||
| if !ok { | ||
| return nil | ||
| } | ||
|
|
||
| workflowDispatchMap, ok := onMap["workflow_dispatch"].(map[string]any) | ||
| if !ok { | ||
| return nil | ||
| } | ||
|
|
||
| inputsMap, ok := workflowDispatchMap["inputs"].(map[string]any) | ||
| if !ok { | ||
| return nil | ||
| } | ||
|
|
||
| for inputName, inputDef := range inputsMap { | ||
| inputDefMap, ok := inputDef.(map[string]any) | ||
| if !ok { | ||
| continue | ||
| } | ||
|
|
||
| required, ok := inputDefMap["required"].(bool) | ||
| if ok && required { | ||
| var triggerNames []string | ||
| if hasSlashCommand { | ||
| triggerNames = append(triggerNames, "slash_command") | ||
| } | ||
| if hasLabelCommand { | ||
| triggerNames = append(triggerNames, "label_command") | ||
| } | ||
| triggerNamesPhrase := strings.Join(triggerNames, " and ") | ||
|
|
||
| return fmt.Errorf( | ||
| "on.workflow_dispatch.inputs.%s.required: true is not allowed when using %s; set required: false", | ||
| inputName, triggerNamesPhrase, |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot review all comments |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in b93490c. I fixed the agentic workflows that hit the new validation ( |
Reviewed all comments. The actionable items are addressed in a000647 (compiler now errors on |
Centralized slash-command workflows should not be blocked by
workflow_dispatchinput requirements, because they are invoked by the router, not manual callers. This change ensures compiled dispatch schemas are compatible with routed slash-command execution.Compiler behavior (centralized slash commands)
applyDefaultscommand-trigger path to normalizeon.workflow_dispatch.inputs.*.requiredtofalsewhenon.slash_command.strategy: centralizedis active.setWorkflowDispatchInputsOptional(...)to safely no-op unlessworkflow_dispatch.inputsis a valid map.Regression coverage
required: trueis emitted asrequired: falsein compiled lock YAML for centralized slash-command workflows.