From cd24669b65fceacfc8bf261331647cc5796c86b4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Apr 2026 23:57:41 +0000 Subject: [PATCH 1/5] Initial plan From e433af702856407e6df41eedd1f88f9dba175a61 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Apr 2026 00:19:25 +0000 Subject: [PATCH 2/5] feat: add compile-time warning for slash_command + bots conflict (#issue) Agent-Logs-Url: https://github.com/github/gh-aw/sessions/063087cd-de65-4d57-ba91-37d72c50058f Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/compiler_validators.go | 14 ++ .../slash_command_bots_warning_test.go | 150 ++++++++++++++++++ 2 files changed, 164 insertions(+) create mode 100644 pkg/workflow/slash_command_bots_warning_test.go diff --git a/pkg/workflow/compiler_validators.go b/pkg/workflow/compiler_validators.go index d01cc7f0982..b7f4df9a680 100644 --- a/pkg/workflow/compiler_validators.go +++ b/pkg/workflow/compiler_validators.go @@ -367,6 +367,20 @@ func (c *Compiler) validateToolConfiguration(workflowData *WorkflowData, markdow c.IncrementWarningCount() } + // Warn when slash_command and bots are both configured: the workflow may be triggered + // by bot activity (e.g. copilot[bot] opening a PR) before the user issues the slash + // command, causing the manual invocation to be ignored while the bot-triggered run is + // still in progress. + if len(workflowData.Command) > 0 && len(workflowData.Bots) > 0 { + fmt.Fprintln(os.Stderr, formatCompilerMessage(markdownPath, "warning", + "Both slash_command and bots triggers are configured. Bot activity "+ + "(e.g., copilot[bot]) matching slash_command events will start the workflow "+ + "automatically, preventing manual slash command invocations while that run is "+ + "in progress. To ensure the workflow only runs on explicit slash commands, "+ + "remove the 'bots:' field.")) + c.IncrementWarningCount() + } + // Inform users when this workflow is a redirect stub for updates. if workflowData.Redirect != "" { fmt.Fprintln(os.Stderr, formatCompilerMessage(markdownPath, "info", diff --git a/pkg/workflow/slash_command_bots_warning_test.go b/pkg/workflow/slash_command_bots_warning_test.go new file mode 100644 index 00000000000..3a6a276ccdc --- /dev/null +++ b/pkg/workflow/slash_command_bots_warning_test.go @@ -0,0 +1,150 @@ +//go:build integration + +package workflow + +import ( + "bytes" + "io" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/github/gh-aw/pkg/testutil" +) + +// TestSlashCommandBotsWarning tests that combining slash_command and bots triggers +// emits a compile-time warning about the potential conflict. +func TestSlashCommandBotsWarning(t *testing.T) { + tests := []struct { + name string + content string + expectWarning bool + }{ + { + name: "slash_command with bots produces conflict warning", + content: `--- +on: + slash_command: + name: rust-review + events: [pull_request, pull_request_comment] + bots: + - "copilot[bot]" +engine: copilot +permissions: + contents: read + pull-requests: read + issues: read +--- + +# Rust Review Workflow +Review Rust code on demand. +`, + expectWarning: true, + }, + { + name: "slash_command without bots does not produce warning", + content: `--- +on: + slash_command: + name: rust-review + events: [pull_request, pull_request_comment] +engine: copilot +permissions: + contents: read + pull-requests: read + issues: read +--- + +# Rust Review Workflow +Review Rust code on demand. +`, + expectWarning: false, + }, + { + name: "bots without slash_command does not produce warning", + content: `--- +on: + pull_request: + types: [opened] + bots: + - "dependabot[bot]" +engine: copilot +permissions: + contents: read + pull-requests: read +--- + +# Dependabot Workflow +Handle dependabot PRs. +`, + expectWarning: false, + }, + { + name: "slash_command with multiple bots produces conflict warning", + content: `--- +on: + slash_command: + name: review + bots: + - "copilot[bot]" + - "renovate[bot]" +engine: copilot +permissions: + contents: read + pull-requests: read + issues: read +--- + +# Review Workflow +Review code on demand. +`, + expectWarning: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tmpDir := testutil.TempDir(t, "slash-command-bots-warning-test") + + testFile := filepath.Join(tmpDir, "test-workflow.md") + if err := os.WriteFile(testFile, []byte(tt.content), 0644); err != nil { + t.Fatal(err) + } + + oldStderr := os.Stderr + r, w, _ := os.Pipe() + os.Stderr = w + + compiler := NewCompiler() + compiler.SetStrictMode(false) + err := compiler.CompileWorkflow(testFile) + + w.Close() + os.Stderr = oldStderr + var buf bytes.Buffer + io.Copy(&buf, r) + stderrOutput := buf.String() + + if err != nil { + t.Errorf("expected compilation to succeed but it failed: %v", err) + return + } + + expectedPhrase := "Both slash_command and bots triggers are configured" + if tt.expectWarning { + if !strings.Contains(stderrOutput, expectedPhrase) { + t.Errorf("expected warning containing %q, got stderr:\n%s", expectedPhrase, stderrOutput) + } + if compiler.GetWarningCount() == 0 { + t.Error("expected warning count > 0 but got 0") + } + return + } + + if strings.Contains(stderrOutput, expectedPhrase) { + t.Errorf("did not expect warning %q, but got stderr:\n%s", expectedPhrase, stderrOutput) + } + }) + } +} From ae4aff7c8f4950edc528a4c6b05154ea2c82281d Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 29 Apr 2026 01:19:29 +0000 Subject: [PATCH 3/5] docs(adr): add draft ADR-29008 for compile-time warning on slash_command + bots conflict Co-Authored-By: Claude Sonnet 4.6 --- ...ime-warning-slash-command-bots-conflict.md | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 docs/adr/29008-compile-time-warning-slash-command-bots-conflict.md diff --git a/docs/adr/29008-compile-time-warning-slash-command-bots-conflict.md b/docs/adr/29008-compile-time-warning-slash-command-bots-conflict.md new file mode 100644 index 00000000000..ae39ef8bb7e --- /dev/null +++ b/docs/adr/29008-compile-time-warning-slash-command-bots-conflict.md @@ -0,0 +1,74 @@ +# ADR-29008: Compile-Time Warning for slash_command + bots Conflict + +**Date**: 2026-04-29 +**Status**: Draft +**Deciders**: pelikhan, copilot-swe-agent + +--- + +## Part 1 — Narrative (Human-Friendly) + +### Context + +Agentic workflow frontmatter supports two independent trigger mechanisms: `slash_command` (explicit manual invocation by a user) and `bots` (automatic invocation when a named bot actor performs a matching event). When both are configured in the same workflow, bot activity — such as `copilot[bot]` opening a pull request — can match the `slash_command` event types (e.g., `pull_request`, `pull_request_comment`) and start the workflow automatically. This causes any subsequent manual slash command invocation to be silently ignored while the bot-triggered run is already in progress. The failure mode is invisible to the workflow author at authoring time and difficult to diagnose at runtime. + +### Decision + +We will emit a non-fatal compile-time warning in `validateToolConfiguration` whenever a workflow's parsed frontmatter contains both a non-empty `slash_command` (via `workflowData.Command`) and a non-empty `bots` list (via `workflowData.Bots`). The warning message names the conflict, provides a concrete example of how it manifests, and directs the author to remove the `bots:` field if exclusively slash-command-driven execution is intended. Using a warning rather than a hard error preserves backward compatibility for any workflow that intentionally wants both triggers. + +### Alternatives Considered + +#### Alternative 1: Hard Compiler Error + +Reject compilation entirely when both `slash_command` and `bots` are present. This is more forceful and guarantees the conflict never reaches production, but it is a breaking change for any existing workflows that may have intentionally paired the triggers for a dual-invocation use case. A hard error would require all such authors to migrate immediately, with no grace period. + +#### Alternative 2: Documentation-Only + +Document the conflict in the workflow authoring guide without any code change. This is the least disruptive approach, but it provides no signal at the point of workflow authoring and leaves authors who have not read the documentation entirely exposed to the silent failure mode. + +#### Alternative 3: Auto-Remove bots: at Compile Time + +Silently strip the `bots:` field from the compiled output when `slash_command` is also present. This eliminates the conflict without requiring author action, but it modifies the workflow against the author's explicit configuration without consent or explanation, which is worse than the original problem. + +### Consequences + +#### Positive +- Authors receive an actionable, human-readable warning at compile time — before the workflow is deployed — rather than discovering the issue through silent runtime failures. +- The warning message includes both the root cause and the recommended fix, minimizing the debugging burden. + +#### Negative +- The warning is non-fatal and can be ignored; a deployed workflow with both triggers will still exhibit the conflict if the author does not act on the warning. +- The check must be updated if the semantics of `slash_command` event routing change (e.g., if `slash_command` is decoupled from `pull_request` event types in a future compiler revision). + +#### Neutral +- The warning is emitted to `stderr` using the existing `formatCompilerMessage` infrastructure, consistent with all other compiler diagnostics. +- Warning count is incremented via `IncrementWarningCount()`, so tooling that surfaces the aggregate warning count will reflect the new check without further changes. + +--- + +## Part 2 — Normative Specification (RFC 2119) + +> The key words **MUST**, **MUST NOT**, **REQUIRED**, **SHALL**, **SHALL NOT**, **SHOULD**, **SHOULD NOT**, **RECOMMENDED**, **MAY**, and **OPTIONAL** in this section are to be interpreted as described in [RFC 2119](https://www.rfc-editor.org/rfc/rfc2119). + +### Conflict Detection + +1. The compiler **MUST** emit a warning diagnostic when a workflow's parsed frontmatter contains both a non-empty `slash_command` trigger (i.e., `workflowData.Command` is non-empty) and a non-empty `bots` list (i.e., `workflowData.Bots` has at least one entry). +2. The warning **MUST** be written to `stderr` using the `formatCompilerMessage` helper with severity `"warning"`. +3. The warning **MUST** increment the compiler's warning counter via `IncrementWarningCount()`. +4. Compilation **MUST NOT** fail (i.e., return a non-nil error) solely because this conflict is detected; the check **MUST** be non-fatal. +5. The warning message **MUST** identify the conflict, describe the failure mode (bot activity starting the workflow before a manual slash command), and provide a remediation step (removing the `bots:` field). + +### Conflict Scope + +1. The check **MUST** fire if and only if both conditions are true simultaneously: `len(workflowData.Command) > 0` AND `len(workflowData.Bots) > 0`. +2. A workflow with `slash_command` and no `bots` entries **MUST NOT** trigger the warning. +3. A workflow with `bots` entries and no `slash_command` **MUST NOT** trigger the warning. +4. The number of entries in `workflowData.Bots` (one vs. multiple) **MUST NOT** affect whether the warning fires; the warning **MUST** fire for any non-zero `bots` list size when `slash_command` is also present. + +### Conformance + +An implementation is considered conformant with this ADR if it satisfies all **MUST** and **MUST NOT** requirements above. Failure to meet any **MUST** or **MUST NOT** requirement constitutes non-conformance. In particular, converting the warning to a hard error or suppressing the `IncrementWarningCount()` call both constitute non-conformance. + +--- + +*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/25085998674) workflow. The PR author must review, complete, and finalize this document before the PR can merge.* From 8e82c88a0a598d8119b16ad51a794d46002d9ba4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Apr 2026 01:33:37 +0000 Subject: [PATCH 4/5] fix: handle os.Pipe error and restore os.Stderr via t.Cleanup in test Agent-Logs-Url: https://github.com/github/gh-aw/sessions/918651d3-e28b-434c-8cc1-51c68f067ba1 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../slash_command_bots_warning_test.go | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/pkg/workflow/slash_command_bots_warning_test.go b/pkg/workflow/slash_command_bots_warning_test.go index 3a6a276ccdc..9d88144f8ca 100644 --- a/pkg/workflow/slash_command_bots_warning_test.go +++ b/pkg/workflow/slash_command_bots_warning_test.go @@ -113,17 +113,29 @@ Review code on demand. } oldStderr := os.Stderr - r, w, _ := os.Pipe() + r, w, pipeErr := os.Pipe() + if pipeErr != nil { + t.Fatal(pipeErr) + } os.Stderr = w + t.Cleanup(func() { + os.Stderr = oldStderr + _ = w.Close() + _ = r.Close() + }) compiler := NewCompiler() compiler.SetStrictMode(false) err := compiler.CompileWorkflow(testFile) - w.Close() + if err := w.Close(); err != nil { + t.Fatal(err) + } os.Stderr = oldStderr var buf bytes.Buffer - io.Copy(&buf, r) + if _, err := io.Copy(&buf, r); err != nil { + t.Fatal(err) + } stderrOutput := buf.String() if err != nil { From 6e7ba1b75357d1581c6e288e19fa5bef9c820c3b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Apr 2026 05:22:11 +0000 Subject: [PATCH 5/5] fix: correct slash_command+bots warning to reflect command-text requirement Agent-Logs-Url: https://github.com/github/gh-aw/sessions/7489c959-1896-4849-82b3-e2a2b047a754 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/compiler_validators.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/workflow/compiler_validators.go b/pkg/workflow/compiler_validators.go index b7f4df9a680..cbc90359768 100644 --- a/pkg/workflow/compiler_validators.go +++ b/pkg/workflow/compiler_validators.go @@ -367,17 +367,17 @@ func (c *Compiler) validateToolConfiguration(workflowData *WorkflowData, markdow c.IncrementWarningCount() } - // Warn when slash_command and bots are both configured: the workflow may be triggered - // by bot activity (e.g. copilot[bot] opening a PR) before the user issues the slash - // command, causing the manual invocation to be ignored while the bot-triggered run is - // still in progress. + // Warn when slash_command and bots are both configured: if a bot listed in bots: posts + // a comment that starts with the slash command text (e.g. /command-name), the + // check_command_position check will pass and the bot will trigger the workflow — + // occupying the concurrency slot and potentially blocking a simultaneous manual invocation. if len(workflowData.Command) > 0 && len(workflowData.Bots) > 0 { fmt.Fprintln(os.Stderr, formatCompilerMessage(markdownPath, "warning", - "Both slash_command and bots triggers are configured. Bot activity "+ - "(e.g., copilot[bot]) matching slash_command events will start the workflow "+ - "automatically, preventing manual slash command invocations while that run is "+ - "in progress. To ensure the workflow only runs on explicit slash commands, "+ - "remove the 'bots:' field.")) + "Both slash_command and bots triggers are configured. If a bot listed in bots: "+ + "posts a comment that starts with the slash command text (e.g., /command-name), "+ + "it will trigger the workflow and occupy the concurrency slot, potentially "+ + "blocking simultaneous manual invocations. To ensure the workflow only runs on "+ + "explicit user commands, remove the 'bots:' field.")) c.IncrementWarningCount() }