diff --git a/.github/workflows/delight.md b/.github/workflows/delight.md index 21c4f794c58..e27d56b8247 100644 --- a/.github/workflows/delight.md +++ b/.github/workflows/delight.md @@ -534,7 +534,7 @@ For additional examples, see the [tools documentation](/tools/overview). **Why Better**: Provides concrete example inline, eliminates need to search elsewhere, includes navigation link for deeper information. ### Good Example: CLI Help Text (Single File) -**File**: `pkg/cli/compile_command.go` +**File**: `pkg/cli/compile_orchestrator.go` **Before**: "Compile workflow files" diff --git a/pkg/cli/README.md b/pkg/cli/README.md index 7fb6956e1fe..9d9ae50143c 100644 --- a/pkg/cli/README.md +++ b/pkg/cli/README.md @@ -17,7 +17,7 @@ All diagnostic output MUST go to `stderr` using `console` formatting helpers. St | `gh aw add` | `NewAddCommand` | Add remote or local workflows to the repository | | `gh aw add-wizard` | `NewAddWizardCommand` | Interactive wizard for adding workflows | | `gh aw new` | `newCmd` (main.go) | Create a new workflow file (supports `--force`, `--interactive`, `--engine`) | -| `gh aw compile` | (compile_command.go) | Compile `.md` workflow files into GitHub Actions `.lock.yml` | +| `gh aw compile` | Cobra `compileCmd` (`cmd/gh-aw/main.go`); orchestration via `CompileWorkflows` (`compile_orchestrator.go`) | Compile `.md` workflow files into GitHub Actions `.lock.yml` | | `gh aw enable` | `enableCmd` (main.go) | Enable a workflow | | `gh aw disable` | `disableCmd` (main.go) | Disable a workflow | | `gh aw run` | `RunWorkflowOnGitHub` (main.go) | Dispatch and monitor workflow runs | diff --git a/pkg/cli/compile_command.go b/pkg/cli/compile_command.go deleted file mode 100644 index 041841540e1..00000000000 --- a/pkg/cli/compile_command.go +++ /dev/null @@ -1,9 +0,0 @@ -package cli - -// This file is intentionally minimal. The compile functionality has been split into: -// - compile_config.go: Configuration types -// - compile_helpers.go: Utility functions -// - compile_validation.go: Validation logic -// - compile_watch.go: Watch mode functionality -// - compile_campaign.go: Campaign validation -// - compile_orchestrator.go: Main orchestration (CompileWorkflows function) diff --git a/pkg/cli/compile_file_operations.go b/pkg/cli/compile_file_operations.go index b068a1cd627..9af6070234a 100644 --- a/pkg/cli/compile_file_operations.go +++ b/pkg/cli/compile_file_operations.go @@ -10,7 +10,7 @@ // - Are used by multiple compile command variants (compile, watch, campaign) // - Provide common compilation patterns and error handling // - Have a clear domain focus (compilation operations) -// - Keep the main compile_command.go file focused on CLI interaction +// - Keep the compile orchestrator focused on CLI interaction // // This follows the helper file conventions documented in skills/developer/SKILL.md. // diff --git a/pkg/workflow/close_entity_helpers.go b/pkg/workflow/close_entity_helpers.go index 812c564a176..cce6c910d24 100644 --- a/pkg/workflow/close_entity_helpers.go +++ b/pkg/workflow/close_entity_helpers.go @@ -13,8 +13,21 @@ // - Follow a consistent entity registry pattern // - Enable DRY principles for close operations // -// This follows the helper file conventions documented in the developer instructions. -// See skills/developer/SKILL.md#helper-file-conventions for details. +// # Why Grouped Here vs. Split Like Update-Entity Files +// +// The update-entity operations (update_issue_helpers.go, +// update_discussion_helpers.go, update_pull_request_helpers.go) are split +// into one file per entity type because each file owns a distinct type +// definition (UpdateIssuesConfig, UpdateDiscussionsConfig, +// UpdatePullRequestsConfig) with different fields per entity. +// +// Close-entity operations share a single CloseEntityConfig struct and use +// a registry pattern (closeEntityDefinition / closeEntityRegistry) to +// express per-entity variation via data rather than per-entity functions. +// Grouping all three entity parsers in one file therefore keeps the registry +// and its consumers together, reducing indirection without sacrificing +// clarity. If a future close-entity type requires a distinct config struct, +// follow the update-entity convention and extract it to its own file. // // # Key Functions // diff --git a/scratchpad/cli-command-patterns.md b/scratchpad/cli-command-patterns.md index 0b32fb8533e..3800728a2fe 100644 --- a/scratchpad/cli-command-patterns.md +++ b/scratchpad/cli-command-patterns.md @@ -137,11 +137,9 @@ For commands with > 500 lines, split into focused files: - `command_name_orchestrator.go` - Main orchestration **Example**: Compile command split into multiple files: -- `compile_command.go` - Command definition (minimal) - `compile_config.go` - Configuration types -- `compile_helpers.go` - Utility functions - `compile_validation.go` - Validation logic -- `compile_orchestrator.go` - Main orchestration +- `compile_orchestrator.go` - Main orchestration (entry point) --- @@ -164,7 +162,7 @@ Format: `cli:command_name` ```go // ✅ CORRECT var auditLog = logger.New("cli:audit") -var compileLog = logger.New("cli:compile_command") +var compileLog = logger.New("cli:compile_orchestrator") var statusLog = logger.New("cli:status_command") // ❌ INCORRECT @@ -723,10 +721,9 @@ if err != nil { ```go // ❌ INCORRECT - Single file with 2000+ lines -compile_command.go // 2000 lines +compile_orchestrator.go // 2000 lines // ✅ CORRECT - Split into focused files -compile_command.go // Command definition (50 lines) compile_config.go // Configuration (100 lines) compile_validation.go // Validation (300 lines) compile_orchestrator.go // Main logic (500 lines) @@ -741,7 +738,7 @@ var logger = logger.New("compile") // ✅ CORRECT var auditLog = logger.New("cli:audit") -var compileLog = logger.New("cli:compile_command") +var compileLog = logger.New("cli:compile_orchestrator") ``` #### 6. Don't Skip Input Validation diff --git a/skills/developer/SKILL.md b/skills/developer/SKILL.md index 98135e686d4..d51695d1398 100644 --- a/skills/developer/SKILL.md +++ b/skills/developer/SKILL.md @@ -12,6 +12,7 @@ This document consolidates technical specifications and development guidelines f - [Capitalization Guidelines](#capitalization-guidelines) - [Code Organization](#code-organization) + - [WASM Build-Variant Pattern](#wasm-build-variant-pattern) - [Validation Architecture](#validation-architecture) - [Security Best Practices](#security-best-practices) - [Safe Output Messages](#safe-output-messages) @@ -441,6 +442,73 @@ normalized := normalizeWorkflowName(userInput) // Wrong tool! ``` +### WASM Build-Variant Pattern + +Seven files in `pkg/workflow/` provide stub implementations of OS-dependent +features for the WASM compilation target (`GOOS=js GOARCH=wasm`) used by the +gh-aw web playground. Each file is named with the `_wasm.go` suffix (Go's +implicit filename build constraint for `GOARCH=wasm`) **and** carries an +explicit `//go:build js || wasm` tag at line 1: + +``` +pkg/workflow/dependabot_wasm.go +pkg/workflow/docker_validation_wasm.go +pkg/workflow/git_helpers_wasm.go +pkg/workflow/github_cli_wasm.go +pkg/workflow/npm_validation_wasm.go +pkg/workflow/pip_validation_wasm.go +pkg/workflow/repository_features_validation_wasm.go +``` + +Each `_wasm.go` file mirrors the public/package-level function signatures of +its non-WASM counterpart but replaces OS calls (exec, filesystem, network) +with either no-ops or `fmt.Errorf("... not available in Wasm")` returns. + +#### When a `_wasm.go` Stub is Required + +Add a `_wasm.go` stub whenever you add a **new function** to an existing +`_wasm.go`-guarded file (or create a new file that calls OS-level tools at +compile/validation time). Specifically: + +- Functions that call `os/exec` or run external binaries (gh, git, docker, + npm, pip, uv, etc.) +- Functions that read from the real filesystem during compilation +- Functions that perform network I/O at validation time + +Functions that **do not** need a WASM stub: +- Pure data transformations (string manipulation, YAML marshaling) +- Functions that only operate on in-memory data structures +- Functions gated behind `WithSkipValidation(true)` (already excluded at + runtime, but still need to compile) + +#### How to Add a Stub + +1. Identify the non-WASM file (e.g., `github_cli.go`). +2. Open (or create) the corresponding `_wasm.go` file (e.g., + `github_cli_wasm.go`). +3. Ensure the build tag at line 1 is `//go:build js || wasm`. +4. Add a stub with the same signature that returns a zero value and/or an + error: + ```go + func MyNewFunction(args ...string) ([]byte, error) { + return nil, fmt.Errorf("MyNewFunction not available in Wasm") + } + ``` +5. Verify the WASM build still compiles: + ```bash + GOOS=js GOARCH=wasm go build ./pkg/workflow/ + ``` + +#### Known Gap + +`github_cli_wasm.go` currently omits stubs for `enrichGHError`, +`runGHWithSpinnerContext`, `RunGHCombinedContext`, `RunGHWithHost`, and +`SetGHHostEnv`. These are unexported helpers or thin wrappers called only +by the exported `RunGH*` family, which are already stubbed; the compiler +does not reference them directly. This is intentional — avoid adding stubs +for unexported helpers unless the WASM build breaks. + + ## Validation Architecture The validation system ensures workflow configurations are correct, secure, and compatible with GitHub Actions before compilation.