Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions docs/adr/32550-honor-workflows-dir-at-all-path-join-sites.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
# ADR-32550: Route All Workflows-Directory Path Joins Through `constants.GetWorkflowDir()`

**Date**: 2026-05-16
**Status**: Draft
**Deciders**: Unknown (draft generated by Design Decision Gate)

---

## Part 1 — Narrative (Human-Friendly)

### Context

`gh-aw` exposes `GH_AW_WORKFLOWS_DIR` as the canonical knob for overriding the workflows directory (the default `.github/workflows`). A helper, `constants.GetWorkflowDir()`, already encapsulates that resolution. However, several production paths still hardcoded `filepath.Join(".github", "workflows")` directly, so resolution, directory creation, and not-found error messaging diverged from the configured behavior. Users setting the override saw inconsistent results: some commands honored it, others silently fell back to the default. The override contract was therefore only *partially* respected.

### Decision

We will route **every** workflows-directory path construction through `constants.GetWorkflowDir()` rather than hardcoded `filepath.Join(..., ".github", "workflows")` literals. This PR updates the five known sites — `pkg/cli/copilot_setup.go`, `pkg/cli/init.go`, `pkg/workflow/dispatch_workflow_file_resolver.go`, `pkg/workflow/dispatch_workflow_validation.go`, and `pkg/workflow/call_workflow_validation.go` — and adds targeted tests pinning override behavior in each affected runtime path (dispatch resolution, call validation, copilot setup, init maintenance workflow generation).

### Alternatives Considered

#### Alternative 1: Continue case-by-case fixes as users report inconsistencies

Leave the literal `filepath.Join(".github", "workflows")` calls in place and fix them only when a specific bug report comes in. **Rejected**: the helper already exists; the literals are bugs against an already-made decision. Letting them linger means new code can copy the wrong pattern and the override contract keeps drifting.

#### Alternative 2: Add a static-analysis rule forbidding hardcoded `.github/workflows` joins

Introduce a custom linter (or `go vet`-style pass) that flags any `filepath.Join` literal containing `".github", "workflows"` outside the `constants` package. **Considered but deferred**: this is complementary, not a substitute. The actual bug fixes have to land first; an enforcement rule on a still-broken codebase would just produce a long ignore list. A follow-up ADR could add the lint once the tree is clean.

### Consequences

#### Positive
- `GH_AW_WORKFLOWS_DIR` is now honored consistently across dispatch resolution, call-workflow validation, copilot setup, and init maintenance generation.
- Error messages reflect the *configured* workflows directory rather than always claiming `.github/workflows`, which previously misled users running with an override.
- Each affected runtime path now has a regression test pinning the override behavior, so future refactors will catch reintroductions.

#### Negative
- The validation packages (`pkg/workflow/{dispatch,call}_workflow_validation.go`) now import `pkg/constants`, coupling them to env-driven configuration; previously they were free of that dependency.
- The "To fix" instructions inside the not-found error message still hardcode the literal `.github/workflows/` string. The path shown to the user is now correct, but the remediation hint is still default-flavored. [TODO: verify whether this is intentional or should also be parameterized.]

#### Neutral
- No new public API or configuration surface — this aligns existing code with an existing helper.
- Tests use `t.Setenv` to scope the override per-test, so they do not need additional infrastructure.

---

## 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).

### Workflows-Directory Path Construction

1. Code that needs to reference the workflows directory **MUST** obtain its name via `constants.GetWorkflowDir()`.
2. Code **MUST NOT** introduce new occurrences of `filepath.Join(..., ".github", "workflows")` (or equivalent string concatenation) for the purpose of locating the workflows directory.
3. Code **MUST NOT** read `GH_AW_WORKFLOWS_DIR` directly outside the `constants` package; all reads **SHALL** go through `constants.GetWorkflowDir()`.
4. Tests that exercise workflows-directory resolution **SHOULD** set `GH_AW_WORKFLOWS_DIR` via `t.Setenv` and verify both that the override path is used and that the default path is *not* created.

### Error Messaging

1. Not-found error messages that include the searched workflows directory path **MUST** print the value returned by `constants.GetWorkflowDir()`, not the literal `.github/workflows`.
2. Remediation text embedded in error messages **MAY** continue to reference `.github/workflows` as the default location for guidance purposes.

### 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.

---

*This is a DRAFT ADR generated by the Design Decision Gate workflow ([run §25963355684](https://github.com/github/gh-aw/actions/runs/25963355684)). The PR author must review, complete, and finalize this document before the PR can merge.*
4 changes: 2 additions & 2 deletions pkg/cli/copilot_setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,8 @@ func ensureCopilotSetupStepsWithUpgrade(ctx context.Context, verbose bool, actio
resolver = workflow.NewActionResolver(cache)
}

// Create .github/workflows directory if it doesn't exist
workflowsDir := filepath.Join(".github", "workflows")
// Create workflow directory if it doesn't exist.
workflowsDir := constants.GetWorkflowDir()
if err := os.MkdirAll(workflowsDir, constants.DirPermPublic); err != nil {
return fmt.Errorf("failed to create workflows directory: %w", err)
}
Expand Down
30 changes: 30 additions & 0 deletions pkg/cli/copilot_setup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -613,6 +613,36 @@ func TestEnsureCopilotSetupSteps_CreateWithDevMode(t *testing.T) {
}
}

func TestEnsureCopilotSetupSteps_UsesWorkflowDirEnvOverride(t *testing.T) {
tmpDir := t.TempDir()
originalDir, err := os.Getwd()
if err != nil {
t.Fatalf("Failed to get current directory: %v", err)
}
defer func() { _ = os.Chdir(originalDir) }()

if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("Failed to change to temp directory: %v", err)
}

t.Setenv("GH_AW_WORKFLOWS_DIR", filepath.Join("custom", "dir"))

err = ensureCopilotSetupSteps(context.Background(), false, workflow.ActionModeDev, "dev")
if err != nil {
t.Fatalf("ensureCopilotSetupSteps(context.Background()) failed: %v", err)
}

overridePath := filepath.Join("custom", "dir", "copilot-setup-steps.yml")
if _, err := os.Stat(overridePath); err != nil {
t.Fatalf("Expected %s to exist: %v", overridePath, err)
}

defaultPath := filepath.Join(".github", "workflows", "copilot-setup-steps.yml")
if _, err := os.Stat(defaultPath); !os.IsNotExist(err) {
t.Fatalf("Expected default path %s to not exist when override is set", defaultPath)
}
}

// TestEnsureCopilotSetupSteps_UpdateExistingWithReleaseMode tests updating an existing file with release mode
func TestEnsureCopilotSetupSteps_UpdateExistingWithReleaseMode(t *testing.T) {
tmpDir := t.TempDir()
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ func ensureMaintenanceWorkflow(ctx context.Context, verbose bool) error {
}

// Determine the workflows directory
workflowsDir := filepath.Join(gitRoot, ".github", "workflows")
workflowsDir := filepath.Join(gitRoot, constants.GetWorkflowDir())
if _, err := os.Stat(workflowsDir); os.IsNotExist(err) {
// No workflows directory yet, skip maintenance workflow generation
initLog.Print("No workflows directory found, skipping maintenance workflow generation")
Expand Down
58 changes: 58 additions & 0 deletions pkg/cli/init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,64 @@ This is a test workflow.
}
}

func TestEnsureMaintenanceWorkflow_UsesWorkflowDirEnvOverride(t *testing.T) {
tempDir := testutil.TempDir(t, "test-maintenance-override-*")

oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("Failed to get current directory: %v", err)
}
defer func() {
_ = os.Chdir(oldWd)
}()
err = os.Chdir(tempDir)
if err != nil {
t.Fatalf("Failed to change directory: %v", err)
}

if err := exec.Command("git", "init").Run(); err != nil {
t.Fatalf("Failed to init git repo: %v", err)
}

t.Setenv("GH_AW_WORKFLOWS_DIR", filepath.Join("custom", "workflows"))

overrideDir := filepath.Join(tempDir, "custom", "workflows")
if err := os.MkdirAll(overrideDir, 0755); err != nil {
t.Fatalf("Failed to create override workflows directory: %v", err)
}

workflowContent := `---
on:
issues:
types: [opened]
safe-outputs:
create-discussion:
expires: 168
---

# Test Workflow
`
workflowPath := filepath.Join(overrideDir, "test-workflow.md")
if err := os.WriteFile(workflowPath, []byte(workflowContent), 0644); err != nil {
t.Fatalf("Failed to create test workflow: %v", err)
}

err = ensureMaintenanceWorkflow(context.Background(), false)
if err != nil {
t.Fatalf("ensureMaintenanceWorkflow returned error: %v", err)
}

overrideMaintenance := filepath.Join(overrideDir, "agentics-maintenance.yml")
if _, err := os.Stat(overrideMaintenance); err != nil {
t.Fatalf("Expected maintenance workflow at override path %s: %v", overrideMaintenance, err)
}

defaultMaintenance := filepath.Join(tempDir, ".github", "workflows", "agentics-maintenance.yml")
if _, err := os.Stat(defaultMaintenance); !os.IsNotExist(err) {
t.Fatalf("Expected default maintenance path %s to not exist when override is set", defaultMaintenance)
}
}

func TestIsGHESHost(t *testing.T) {
tests := []struct {
Host string
Expand Down
5 changes: 3 additions & 2 deletions pkg/workflow/call_workflow_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"os"
"path/filepath"

"github.com/github/gh-aw/pkg/constants"
"github.com/github/gh-aw/pkg/parser"
"github.com/goccy/go-yaml"
)
Expand Down Expand Up @@ -72,9 +73,9 @@ func (c *Compiler) validateCallWorkflow(data *WorkflowData, workflowPath string)
currentDir := filepath.Dir(workflowPath)
githubDir := filepath.Dir(currentDir)
repoRoot := filepath.Dir(githubDir)
workflowsDir := filepath.Join(repoRoot, ".github", "workflows")
workflowsDir := filepath.Join(repoRoot, constants.GetWorkflowDir())

notFoundErr := fmt.Errorf("call-workflow: workflow '%s' not found in %s\n\nChecked for: %s.md, %s.lock.yml, %s.yml\n\nTo fix:\n1. Verify the workflow file exists in .github/workflows/\n2. Ensure the filename matches exactly (case-sensitive)\n3. Use the filename without extension in your configuration", workflowName, workflowsDir, workflowName, workflowName, workflowName)
notFoundErr := fmt.Errorf("call-workflow: workflow '%s' not found in %s\n\nChecked for: %s.md, %s.lock.yml, %s.yml\n\nTo fix:\n1. Verify the workflow file exists in %s/\n2. Ensure the filename matches exactly (case-sensitive)\n3. Use the filename without extension in your configuration", workflowName, workflowsDir, workflowName, workflowName, workflowName, workflowsDir)
if returnErr := collector.Add(notFoundErr); returnErr != nil {
return returnErr
}
Expand Down
42 changes: 42 additions & 0 deletions pkg/workflow/call_workflow_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,48 @@ jobs:
assert.NoError(t, err, "Valid worker with workflow_call should pass validation")
}

func TestValidateCallWorkflow_UsesWorkflowDirEnvOverride(t *testing.T) {
compiler := NewCompiler(WithVersion("1.0.0"))

tmpDir := t.TempDir()
awDir := filepath.Join(tmpDir, ".github", "aw")
overrideWorkflowsDir := filepath.Join(tmpDir, "custom", "workflows")
err := os.MkdirAll(awDir, 0755)
require.NoError(t, err, "Failed to create aw directory")
err = os.MkdirAll(overrideWorkflowsDir, 0755)
require.NoError(t, err, "Failed to create override workflows directory")

t.Setenv("GH_AW_WORKFLOWS_DIR", filepath.Join("custom", "workflows"))

workerContent := `name: Worker A
on:
workflow_call:
jobs:
work:
runs-on: ubuntu-latest
steps:
- run: echo "Working"
`
err = os.WriteFile(filepath.Join(overrideWorkflowsDir, "worker-a.lock.yml"), []byte(workerContent), 0644)
require.NoError(t, err, "Failed to write worker file")

gatewayFile := filepath.Join(awDir, "gateway.md")

workflowData := &WorkflowData{
SafeOutputs: &SafeOutputsConfig{
CallWorkflow: &CallWorkflowConfig{
BaseSafeOutputConfig: BaseSafeOutputConfig{
Max: strPtr("1"),
},
Workflows: []string{"worker-a"},
},
},
}

err = compiler.validateCallWorkflow(workflowData, gatewayFile)
assert.NoError(t, err, "Workflow should resolve from GH_AW_WORKFLOWS_DIR override")
}

// TestValidateCallWorkflow_MDSourceWithWorkflowCall tests that a .md source with workflow_call
// trigger passes validation (same-batch compilation target)
func TestValidateCallWorkflow_MDSourceWithWorkflowCall(t *testing.T) {
Expand Down
11 changes: 6 additions & 5 deletions pkg/workflow/dispatch_workflow_file_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"path/filepath"
"strings"

"github.com/github/gh-aw/pkg/constants"
"github.com/github/gh-aw/pkg/fileutil"
"github.com/github/gh-aw/pkg/parser"
)
Expand Down Expand Up @@ -52,7 +53,7 @@ type findWorkflowFileResult struct {
ymlExists bool
}

// findWorkflowFile searches for a workflow file in .github/workflows directory only
// findWorkflowFile searches for a workflow file in the configured workflows directory only.
// Returns paths and existence flags for .md, .lock.yml, and .yml files
func findWorkflowFile(workflowName string, currentWorkflowPath string) (*findWorkflowFileResult, error) {
dispatchWorkflowValidationLog.Printf("Finding workflow file: name=%s, current_path=%s", workflowName, currentWorkflowPath)
Expand All @@ -61,13 +62,13 @@ func findWorkflowFile(workflowName string, currentWorkflowPath string) (*findWor
// Get the current workflow's directory
currentDir := filepath.Dir(currentWorkflowPath)

// Get repo root by going up from current directory
// Assume structure: <repo-root>/.github/workflows/file.md or <repo-root>/.github/aw/file.md
// Get repo root by going up from the current workflow directory.
// Assume structure: <repo-root>/<configured-workflows-dir>/file.md or <repo-root>/.github/aw/file.md.
githubDir := filepath.Dir(currentDir) // .github
repoRoot := filepath.Dir(githubDir) // repo root

// Only search in .github/workflows (standard GitHub Actions location)
searchDir := filepath.Join(repoRoot, ".github", "workflows")
// Only search in the configured workflows directory.
searchDir := filepath.Join(repoRoot, constants.GetWorkflowDir())
Comment on lines 67 to +71

// Build paths for the workflows directory
mdPath := filepath.Clean(filepath.Join(searchDir, workflowName+".md"))
Expand Down
5 changes: 3 additions & 2 deletions pkg/workflow/dispatch_workflow_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"path/filepath"
"strings"

"github.com/github/gh-aw/pkg/constants"
"github.com/goccy/go-yaml"
)

Expand Down Expand Up @@ -58,8 +59,8 @@ func (c *Compiler) validateDispatchWorkflow(data *WorkflowData, workflowPath str
currentDir := filepath.Dir(workflowPath)
githubDir := filepath.Dir(currentDir)
repoRoot := filepath.Dir(githubDir)
workflowsDir := filepath.Join(repoRoot, ".github", "workflows")
notFoundErr := fmt.Errorf("dispatch-workflow: workflow '%s' not found in %s\n\nChecked for: %s.md, %s.lock.yml, %s.yml\n\nTo fix:\n1. Verify the workflow file exists in .github/workflows/\n2. Ensure the filename matches exactly (case-sensitive)\n3. Use the filename without extension in your configuration", workflowName, workflowsDir, workflowName, workflowName, workflowName)
workflowsDir := filepath.Join(repoRoot, constants.GetWorkflowDir())
notFoundErr := fmt.Errorf("dispatch-workflow: workflow '%s' not found in %s\n\nChecked for: %s.md, %s.lock.yml, %s.yml\n\nTo fix:\n1. Verify the workflow file exists in %s/\n2. Ensure the filename matches exactly (case-sensitive)\n3. Use the filename without extension in your configuration", workflowName, workflowsDir, workflowName, workflowName, workflowName, workflowsDir)
if returnErr := collector.Add(notFoundErr); returnErr != nil {
return returnErr
}
Expand Down
57 changes: 57 additions & 0 deletions pkg/workflow/dispatch_workflow_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,63 @@ safe-outputs:
assert.Contains(t, err.Error(), "workflow 'missing-workflow' not found")
}

func TestDispatchWorkflowValidation_UsesWorkflowDirEnvOverride(t *testing.T) {
compiler := NewCompiler(WithVersion("1.0.0"))

tmpDir := t.TempDir()
awDir := filepath.Join(tmpDir, ".github", "aw")
overrideWorkflowsDir := filepath.Join(tmpDir, "custom", "workflows")

err := os.MkdirAll(awDir, 0755)
require.NoError(t, err, "Failed to create aw directory")
err = os.MkdirAll(overrideWorkflowsDir, 0755)
require.NoError(t, err, "Failed to create override workflows directory")

t.Setenv("GH_AW_WORKFLOWS_DIR", filepath.Join("custom", "workflows"))

dispatcherWorkflow := `---
on: issues
engine: copilot
permissions:
contents: read
safe-outputs:
dispatch-workflow:
workflows:
- target-worker
max: 1
---

# Dispatcher Workflow
`
dispatcherFile := filepath.Join(awDir, "dispatcher.md")
err = os.WriteFile(dispatcherFile, []byte(dispatcherWorkflow), 0644)
require.NoError(t, err, "Failed to write dispatcher workflow")

targetWorkflow := `name: Target Worker
on:
workflow_dispatch:
jobs:
work:
runs-on: ubuntu-latest
steps:
- run: echo "Working"
`
err = os.WriteFile(filepath.Join(overrideWorkflowsDir, "target-worker.lock.yml"), []byte(targetWorkflow), 0644)
require.NoError(t, err, "Failed to write target workflow")

oldDir, err := os.Getwd()
require.NoError(t, err, "Failed to get current directory")
err = os.Chdir(awDir)
require.NoError(t, err, "Failed to change directory")
defer func() { _ = os.Chdir(oldDir) }()

workflowData, err := compiler.ParseWorkflowFile("dispatcher.md")
require.NoError(t, err, "Failed to parse workflow")

err = compiler.validateDispatchWorkflow(workflowData, dispatcherFile)
assert.NoError(t, err, "Workflow should resolve from GH_AW_WORKFLOWS_DIR override")
}

func TestShouldSkipLocalDispatchWorkflowValidation(t *testing.T) {
compiler := NewCompiler(WithVersion("1.0.0"))
compiler.SetRepositorySlug("my-org/my-repo")
Expand Down
Loading