diff --git a/.gitattributes b/.gitattributes index 33e102d50b4..c80e1a7bf05 100644 --- a/.gitattributes +++ b/.gitattributes @@ -4,7 +4,8 @@ .github/aw/github-agentic-workflows.md linguist-generated=true merge=ours pkg/cli/workflows/*.lock.yml linguist-generated=true merge=ours pkg/workflow/js/*.js linguist-generated=true +pkg/workflow/js/*.cjs linguist-generated=true +pkg/workflow/sh/*.sh linguist-generated=true actions/*/index.js linguist-generated=true -actions/setup/js/*.cjs linguist-generated=true .github/workflows/*.campaign.g.md linguist-generated=true merge=ours diff --git a/AGENTS.md b/AGENTS.md index d4eac2f5d9a..c76d0be84e5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -112,6 +112,58 @@ make build # ~1.5s ./gh-aw --help ``` +## Build System + +### Shell Script Sync + +**ALWAYS sync shell scripts before building:** + +Shell scripts in `actions/setup/sh/` are the **source of truth** and are automatically synced to `pkg/workflow/sh/` during the build process. + +```bash +make sync-shell-scripts # Copies actions/setup/sh/*.sh → pkg/workflow/sh/ +make build # Automatically runs sync-shell-scripts +``` + +**When modifying shell scripts:** +1. Edit files in `actions/setup/sh/` (source of truth) +2. Run `make build` (automatically syncs to pkg/workflow/sh/) +3. The synced files in `pkg/workflow/sh/` are embedded in the binary via `//go:embed` +4. **Never** edit files in `pkg/workflow/sh/` directly - they are generated + +**Key points:** +- `actions/setup/sh/*.sh` = Source of truth (manually edited) +- `pkg/workflow/sh/*.sh` = Generated (copied during build, marked as linguist-generated) +- The build process: `actions/setup/sh/` → `pkg/workflow/sh/` → embedded in binary + +### JavaScript File Sync + +**JavaScript files follow the SAME pattern as shell scripts:** + +JavaScript files in `actions/setup/js/` are the **source of truth** and are automatically synced to `pkg/workflow/js/` during the build process. + +```bash +make sync-js-scripts # Copies actions/setup/js/*.cjs → pkg/workflow/js/ +make build # Automatically runs sync-js-scripts +``` + +**When modifying JavaScript files:** +1. Edit files in `actions/setup/js/` (source of truth) +2. Run `make build` (automatically syncs to pkg/workflow/js/) +3. The synced files in `pkg/workflow/js/` are embedded in the binary via `//go:embed` +4. **Never** edit production files in `pkg/workflow/js/` directly - they are generated +5. Test files (*.test.cjs) remain only in `pkg/workflow/js/` and are not synced + +**Key points:** +- `actions/setup/js/*.cjs` = Source of truth (manually edited, production files only) +- `pkg/workflow/js/*.cjs` = Generated (copied during build, marked as linguist-generated) +- `pkg/workflow/js/*.test.cjs` = Test files (remain in pkg/workflow/js/, not synced) +- The build process: `actions/setup/js/` → `pkg/workflow/js/` → embedded in binary + +**Summary of patterns:** +- Shell scripts: `actions/setup/sh/` (source) → `pkg/workflow/sh/` (generated) +- JavaScript: `actions/setup/js/` (source) → `pkg/workflow/js/` (generated) + ## Development Workflow ### Build & Test Commands diff --git a/Makefile b/Makefile index fec9603d820..d16b4328ebf 100644 --- a/Makefile +++ b/Makefile @@ -14,7 +14,7 @@ all: build build-awmg # Build the binary, run make deps before this .PHONY: build -build: sync-templates sync-action-pins +build: sync-templates sync-action-pins sync-shell-scripts sync-js-scripts go build $(LDFLAGS) -o $(BINARY_NAME) ./cmd/gh-aw # Build the awmg (MCP gateway) binary @@ -433,6 +433,23 @@ sync-templates: @cp .github/agents/debug-agentic-workflow.agent.md pkg/cli/templates/ @echo "✓ Templates synced successfully" +# Sync shell scripts from actions/setup/sh to pkg/workflow/sh +.PHONY: sync-shell-scripts +sync-shell-scripts: + @echo "Syncing shell scripts from actions/setup/sh to pkg/workflow/sh..." + @mkdir -p pkg/workflow/sh + @cp actions/setup/sh/*.sh pkg/workflow/sh/ + @echo "✓ Shell scripts synced successfully" + +# Sync JavaScript files from actions/setup/js to pkg/workflow/js +.PHONY: sync-js-scripts +sync-js-scripts: + @echo "Syncing JavaScript files from actions/setup/js to pkg/workflow/js..." + @mkdir -p pkg/workflow/js + @cp actions/setup/js/*.cjs pkg/workflow/js/ + @cp actions/setup/js/*.json pkg/workflow/js/ 2>/dev/null || true + @echo "✓ JavaScript files synced successfully" + # Sync action pins from .github/aw to pkg/workflow/data .PHONY: sync-action-pins sync-action-pins: @@ -567,6 +584,8 @@ help: @echo " install - Install binary locally" @echo " sync-templates - Sync templates from .github to pkg/cli/templates (runs automatically during build)" @echo " sync-action-pins - Sync actions-lock.json from .github/aw to pkg/workflow/data (runs automatically during build)" + @echo " sync-shell-scripts - Sync shell scripts from actions/setup/sh to pkg/workflow/sh (runs automatically during build)" + @echo " sync-js-scripts - Sync JavaScript files from actions/setup/js to pkg/workflow/js (runs automatically during build)" @echo " update - Update GitHub Actions and workflows, sync action pins, and rebuild binary" @echo " fix - Apply automatic codemod-style fixes to workflow files (depends on build)" @echo " recompile - Recompile all workflow files (runs init, depends on build)" diff --git a/actions/setup/js/add_copilot_reviewer.cjs b/actions/setup/js/add_copilot_reviewer.cjs index 4da23077c20..51c5a43a634 100644 --- a/actions/setup/js/add_copilot_reviewer.cjs +++ b/actions/setup/js/add_copilot_reviewer.cjs @@ -17,14 +17,14 @@ const COPILOT_REVIEWER_BOT = "copilot-pull-request-reviewer[bot]"; async function main() { // Validate required environment variables - const prNumberStr = process.env.PR_NUMBER; + const prNumberStr = process.env.PR_NUMBER?.trim(); - if (!prNumberStr || prNumberStr.trim() === "") { + if (!prNumberStr) { core.setFailed("PR_NUMBER environment variable is required but not set"); return; } - const prNumber = parseInt(prNumberStr.trim(), 10); + const prNumber = parseInt(prNumberStr, 10); if (isNaN(prNumber) || prNumber <= 0) { core.setFailed(`Invalid PR_NUMBER: ${prNumberStr}. Must be a positive integer.`); return; @@ -52,7 +52,7 @@ Successfully added Copilot as a reviewer to PR #${prNumber}. ) .write(); } catch (error) { - const errorMessage = error instanceof Error ? error.message : String(error); + const errorMessage = error?.message ?? String(error); core.error(`Failed to add Copilot as reviewer: ${errorMessage}`); core.setFailed(`Failed to add Copilot as reviewer to PR #${prNumber}: ${errorMessage}`); } diff --git a/actions/setup/js/mcp_http_transport.cjs b/actions/setup/js/mcp_http_transport.cjs index 25ceb145fe6..b37a581e4da 100644 --- a/actions/setup/js/mcp_http_transport.cjs +++ b/actions/setup/js/mcp_http_transport.cjs @@ -272,17 +272,14 @@ class MCPHTTPTransport { res.writeHead(200, headers); res.end(JSON.stringify(response)); } catch (error) { - // Log the full error with stack trace on the server for debugging - this.logger.debugError("Error in handleRequest: ", error); if (!res.headersSent) { res.writeHead(500, { "Content-Type": "application/json" }); - // Send a generic error message to the client to avoid exposing stack traces res.end( JSON.stringify({ jsonrpc: "2.0", error: { code: -32603, - message: "Internal server error", + message: error instanceof Error ? error.message : String(error), }, id: null, }) diff --git a/actions/setup/js/safe_inputs_mcp_server_http.cjs b/actions/setup/js/safe_inputs_mcp_server_http.cjs index 2c555d39de7..2ddffc10371 100644 --- a/actions/setup/js/safe_inputs_mcp_server_http.cjs +++ b/actions/setup/js/safe_inputs_mcp_server_http.cjs @@ -220,17 +220,15 @@ async function startHttpServer(configPath, options = {}) { // Let the transport handle the request await transport.handleRequest(req, res, body); } catch (error) { - // Log the full error with stack trace on the server for debugging logger.debugError("Error handling request: ", error); if (!res.headersSent) { res.writeHead(500, { "Content-Type": "application/json" }); - // Send a generic error message to the client to avoid exposing stack traces res.end( JSON.stringify({ jsonrpc: "2.0", error: { code: -32603, - message: "Internal server error", + message: error instanceof Error ? error.message : String(error), }, id: null, }) diff --git a/pkg/cli/actions_build_command.go b/pkg/cli/actions_build_command.go index f8f19969b15..279d5c3a8dc 100644 --- a/pkg/cli/actions_build_command.go +++ b/pkg/cli/actions_build_command.go @@ -129,26 +129,8 @@ func ActionsCleanCommand() error { } } - // Clean js/ and sh/ directories for setup action - if actionName == "setup" { - jsDir := filepath.Join(actionsDir, actionName, "js") - if _, err := os.Stat(jsDir); err == nil { - if err := os.RemoveAll(jsDir); err != nil { - return fmt.Errorf("failed to remove %s: %w", jsDir, err) - } - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf(" ✓ Removed %s/js/", actionName))) - cleanedCount++ - } - - shDir := filepath.Join(actionsDir, actionName, "sh") - if _, err := os.Stat(shDir); err == nil { - if err := os.RemoveAll(shDir); err != nil { - return fmt.Errorf("failed to remove %s: %w", shDir, err) - } - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf(" ✓ Removed %s/sh/", actionName))) - cleanedCount++ - } - } + // For setup action, both js/ and sh/ directories are source of truth (NOT generated) + // Do not clean them } fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("✨ Cleanup complete (%d files removed)", cleanedCount))) @@ -342,65 +324,44 @@ func buildSetupSafeOutputsAction(actionsDir, actionName string) error { return nil } -// buildSetupAction builds the setup action by copying JavaScript files to js/ directory -// and shell scripts to sh/ directory +// buildSetupAction builds the setup action by checking that source files exist. +// Note: Both JavaScript and shell scripts are source of truth in actions/setup/js/ and actions/setup/sh/ +// They get synced to pkg/workflow/js/ and pkg/workflow/sh/ during the build process via Makefile targets. func buildSetupAction(actionsDir, actionName string) error { actionPath := filepath.Join(actionsDir, actionName) jsDir := filepath.Join(actionPath, "js") shDir := filepath.Join(actionPath, "sh") - // Get dependencies for this action - dependencies := getActionDependencies(actionName) - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf(" ✓ Found %d JavaScript dependencies", len(dependencies)))) - - // Get all JavaScript sources - sources := workflow.GetJavaScriptSources() - - // Create js directory if it doesn't exist - if err := os.MkdirAll(jsDir, 0755); err != nil { - return fmt.Errorf("failed to create js directory: %w", err) - } - - // Copy each dependency file to the js directory - copiedCount := 0 - for _, dep := range dependencies { - if content, ok := sources[dep]; ok { - destPath := filepath.Join(jsDir, dep) - if err := os.WriteFile(destPath, []byte(content), 0644); err != nil { - return fmt.Errorf("failed to write %s: %w", dep, err) + // JavaScript files in actions/setup/js/ are the source of truth + if _, err := os.Stat(jsDir); err == nil { + // Count JavaScript files + entries, err := os.ReadDir(jsDir) + if err == nil { + jsCount := 0 + for _, entry := range entries { + if !entry.IsDir() && strings.HasSuffix(entry.Name(), ".cjs") { + jsCount++ + } } - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf(" - %s", dep))) - copiedCount++ - } else { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf(" ⚠ Warning: Could not find %s", dep))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf(" ✓ JavaScript files in js/ (source of truth): %d", jsCount))) } } - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf(" ✓ Copied %d files to js/", copiedCount))) - - // Get bundled shell scripts - shellScripts := workflow.GetBundledShellScripts() - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf(" ✓ Found %d shell scripts", len(shellScripts)))) - - // Create sh directory if it doesn't exist - if err := os.MkdirAll(shDir, 0755); err != nil { - return fmt.Errorf("failed to create sh directory: %w", err) - } - - // Copy each shell script to the sh directory - shCopiedCount := 0 - for filename, content := range shellScripts { - destPath := filepath.Join(shDir, filename) - // Shell scripts should be executable (0755) - if err := os.WriteFile(destPath, []byte(content), 0755); err != nil { - return fmt.Errorf("failed to write %s: %w", filename, err) + // Shell scripts in actions/setup/sh/ are the source of truth + if _, err := os.Stat(shDir); err == nil { + // Count shell scripts + entries, err := os.ReadDir(shDir) + if err == nil { + shCount := 0 + for _, entry := range entries { + if !entry.IsDir() && strings.HasSuffix(entry.Name(), ".sh") { + shCount++ + } + } + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf(" ✓ Shell scripts in sh/ (source of truth): %d", shCount))) } - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf(" - %s", filename))) - shCopiedCount++ } - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf(" ✓ Copied %d shell scripts to sh/", shCopiedCount))) - return nil } diff --git a/specs/actions.md b/specs/actions.md index edf9b19f76b..4a67b54e2be 100644 --- a/specs/actions.md +++ b/specs/actions.md @@ -141,6 +141,14 @@ Create a custom actions system that: gh-aw/ ├── actions/ # Custom GitHub Actions │ ├── README.md # Actions documentation +│ ├── setup/ # Setup action with shell scripts +│ │ ├── action.yml # Action metadata +│ │ ├── setup.sh # Main setup script +│ │ ├── js/ # JavaScript files (generated) +│ │ │ └── *.cjs # Copied from pkg/workflow/js/ +│ │ ├── sh/ # Shell scripts (SOURCE OF TRUTH) +│ │ │ └── *.sh # Manually edited shell scripts +│ │ └── README.md # Action-specific docs │ ├── setup-safe-inputs/ # Safe inputs MCP server setup │ │ ├── action.yml # Action metadata │ │ ├── index.js # Bundled output (committed) @@ -158,17 +166,36 @@ gh-aw/ │ │ └── actions_build_command.go # Build system implementation │ └── workflow/ │ ├── js.go # JavaScript sources map -│ └── js/ # Embedded JavaScript files -│ ├── *.cjs # CommonJS modules -│ └── *.json # JSON configuration files +│ ├── sh.go # Shell script sources map +│ ├── js/ # Embedded JavaScript files +│ │ ├── *.cjs # CommonJS modules +│ │ └── *.json # JSON configuration files +│ └── sh/ # Embedded shell scripts (GENERATED) +│ └── *.sh # Copied from actions/setup/sh/ ├── cmd/gh-aw/ │ └── main.go # CLI entry point with commands -├── Makefile # Build targets -├── .gitattributes # Mark generated files +├── Makefile # Build targets (includes sync-shell-scripts) +├── .gitattributes # Mark generated files (pkg/workflow/sh/*.sh) └── .github/workflows/ └── ci.yml # CI pipeline with actions-build job ``` +**Shell Script Sync Flow:** +1. **Source of Truth**: `actions/setup/sh/*.sh` (manually edited) +2. **Sync Step**: `make sync-shell-scripts` copies to `pkg/workflow/sh/` +3. **Build Step**: `make build` embeds `pkg/workflow/sh/*.sh` via `//go:embed` +4. **Actions Build**: `make actions-build` does NOT copy shell scripts (they already exist in actions/setup/sh/) + +**JavaScript File Sync Flow:** +1. **Source of Truth**: `actions/setup/js/*.cjs` (manually edited, production files only) +2. **Sync Step**: `make sync-js-scripts` copies to `pkg/workflow/js/` +3. **Build Step**: `make build` embeds `pkg/workflow/js/*.cjs` via `//go:embed` +4. **Test Files**: `pkg/workflow/js/*.test.cjs` remain only in pkg/workflow/js/ (not synced) + +Both follow the same pattern now: +- Shell: `actions/setup/sh/` (source) → `pkg/workflow/sh/` (generated) +- JavaScript: `actions/setup/js/` (source) → `pkg/workflow/js/` (generated) + ### Action Structure Each action follows this template: @@ -225,16 +252,50 @@ for (const [filename, content] of Object.entries(FILES)) { The build system is implemented entirely in Go and follows these steps: -1. **Discovery**: Scans `actions/` directory for action subdirectories -2. **Validation**: Validates each `action.yml` file structure -3. **Dependency Resolution**: Maps action name to required JavaScript files (manual mapping in Go) -4. **File Reading**: Retrieves file contents from `workflow.GetJavaScriptSources()` -5. **Bundling**: Creates JSON object with all dependencies -6. **Code Generation**: Replaces `FILES` placeholder in source with bundled content -7. **Output**: Writes bundled `index.js` to action directory +1. **Shell Script Sync** (NEW): Copies shell scripts from `actions/setup/sh/` to `pkg/workflow/sh/` +2. **Discovery**: Scans `actions/` directory for action subdirectories +3. **Validation**: Validates each `action.yml` file structure +4. **Dependency Resolution**: Maps action name to required JavaScript files (manual mapping in Go) +5. **File Reading**: Retrieves file contents from `workflow.GetJavaScriptSources()` +6. **Bundling**: Creates JSON object with all dependencies +7. **Code Generation**: Replaces `FILES` placeholder in source with bundled content +8. **Output**: Writes bundled `index.js` to action directory **Key Point**: The build system is pure Go code in `pkg/cli/actions_build_command.go`. There are no JavaScript build scripts - everything uses the workflow compiler's existing infrastructure. +### Shell Script Sync Process + +**Important**: Shell scripts follow a different pattern than JavaScript files: + +**Shell Scripts** (source in actions/setup/sh/): +``` +actions/setup/sh/*.sh → pkg/workflow/sh/*.sh → Embedded in binary + (SOURCE OF TRUTH) (GENERATED) (go:embed) +``` + +**JavaScript Files** (source in actions/setup/js/): +``` +actions/setup/js/*.cjs → pkg/workflow/js/*.cjs → Embedded in binary + (SOURCE OF TRUTH) (GENERATED) (go:embed) +``` + +Note: Test files (`*.test.cjs`) remain only in `pkg/workflow/js/` and are not synced from actions/setup/js/. + +The sync happens via Makefile targets, which are automatically run as part of `make build`: + +```bash +make sync-shell-scripts # Explicit shell sync +make sync-js-scripts # Explicit JavaScript sync +make build # Includes both syncs +``` + +**Why this pattern?** +- Both shell scripts and JavaScript are part of the setup action itself and live in `actions/setup/` +- They need to be embedded in the Go binary for compilation +- Syncing before build ensures the embedded files match the action's source files +- This creates a single source of truth in the actions directory +- Test files remain in pkg/workflow/js/ for development + ### Build Commands Use Makefile targets for building actions: @@ -533,6 +594,21 @@ The CI runs when: #### Common Tasks +**Modify a shell script**: +1. Edit the file in `actions/setup/sh/` (source of truth) +2. Run `make build` (syncs to pkg/workflow/sh/ and rebuilds binary) +3. Run `make actions-build` (builds actions including setup) +4. Test in a workflow to verify behavior +5. Commit both `actions/setup/sh/*.sh` and generated `pkg/workflow/sh/*.sh` + +**Add a new shell script**: +1. Create the file in `actions/setup/sh/` +2. Add `//go:embed` directive in `pkg/workflow/sh.go` +3. Add to `GetBundledShellScripts()` function in `sh.go` +4. Run `make build` to sync and embed +5. Update setup action's `setup.sh` to use the new script +6. Run `make actions-build` to build the setup action + **Add a new action**: 1. Create directory structure in `actions/` 2. Write `action.yml`, `src/index.js`, `README.md` @@ -557,6 +633,13 @@ The CI runs when: - `pkg/cli/actions_build_command.go` - **Pure Go build system** (no JavaScript) - `internal/tools/actions-build/main.go` - Internal CLI tool dispatcher (development only) - `pkg/workflow/js.go` - JavaScript source map and embedded files +- `pkg/workflow/sh.go` - Shell script source map and embedded files +- `actions/setup/sh/` - **Source of truth for shell scripts** (manually edited) +- `pkg/workflow/sh/` - Generated shell scripts (synced from actions/setup/sh/) +- `Makefile` - Primary interface for building actions (`make actions-build`, `make sync-shell-scripts`) +- `.github/workflows/ci.yml` - CI validation +- `actions/README.md` - Actions documentation +- `.gitattributes` - Marks generated files (pkg/workflow/sh/*.sh, actions/setup/js/*.cjs) - `Makefile` - Primary interface for building actions (`make actions-build`) - `.github/workflows/ci.yml` - CI validation - `actions/README.md` - Actions documentation