diff --git a/.github/workflows/smoke-claude.lock.yml b/.github/workflows/smoke-claude.lock.yml index 05b78d1c6e..ec583e7a1e 100644 --- a/.github/workflows/smoke-claude.lock.yml +++ b/.github/workflows/smoke-claude.lock.yml @@ -33,7 +33,9 @@ # - shared/mcp/tavily.md # - shared/reporting.md # -# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"22441496741c536b8d9d809d2e0e13589a1cf6874aa01ae130f90aabfaa11d11"} +# inlined-imports: true +# +# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"4c1aff44aafcd080af123475aa4736bc98b28c1a12f5775c6374e521799265e9"} name: "Smoke Claude" "on": @@ -217,28 +219,334 @@ jobs: GH_AW_PROMPT_EOF cat << 'GH_AW_PROMPT_EOF' >> "$GH_AW_PROMPT" - {{#runtime-import .github/workflows/shared/mcp-pagination.md}} + ## MCP Response Size Limits + + MCP tool responses have a **25,000 token limit**. When GitHub API responses exceed this limit, workflows must retry with pagination parameters, wasting turns and tokens. + + ### Common Scenarios + + **Problem**: Fetching large result sets without pagination + - `list_pull_requests` with many PRs (75,897 tokens in one case) + - `pull_request_read` with large diff/comments (31,675 tokens observed) + - `search_issues`, `search_code` with many results + + **Solution**: Use proactive pagination to stay under token limits + + ### Pagination Best Practices + + #### 1. Use `perPage` Parameter + + Limit results per request to prevent oversized responses: + + ```bash + # Good: Fetch PRs in small batches + list_pull_requests --perPage 10 + + # Good: Get issue with limited comments + issue_read --method get_comments --perPage 20 + + # Bad: Default pagination may return too much data + list_pull_requests # May exceed 25k tokens + ``` + + #### 2. Common `perPage` Values + + - **10-20**: For detailed items (PRs with diffs, issues with comments) + - **50-100**: For simpler list operations (commits, branches, labels) + - **1-5**: For exploratory queries or schema discovery + + #### 3. Handle Pagination Loops + + When you need all results: + + ```bash + # Step 1: Fetch first page + result=$(list_pull_requests --perPage 20 --page 1) + + # Step 2: Check if more pages exist + # Most list operations return metadata about total count or next page + + # Step 3: Fetch subsequent pages if needed + result=$(list_pull_requests --perPage 20 --page 2) + ``` + + ### Tool-Specific Guidance + + #### Pull Requests + + ```bash + # Fetch recent PRs in small batches + list_pull_requests --state all --perPage 10 --sort updated --direction desc + + # Get PR details without full diff/comments + pull_request_read --method get --pullNumber 123 + + # Get PR files separately if needed + pull_request_read --method get_files --pullNumber 123 --perPage 30 + ``` + + #### Issues + + ```bash + # List issues with pagination + list_issues --perPage 20 --page 1 + + # Get issue comments in batches + issue_read --method get_comments --issue_number 123 --perPage 20 + ``` + + #### Code Search + + ```bash + # Search with limited results + search_code --query "function language:go" --perPage 10 + ``` + + ### Error Messages to Watch For + + If you see these errors, add pagination: + + - `MCP tool "list_pull_requests" response (75897 tokens) exceeds maximum allowed tokens (25000)` + - `MCP tool "pull_request_read" response (31675 tokens) exceeds maximum allowed tokens (25000)` + - `Response too large for tool [tool_name]` + + ### Performance Tips + + 1. **Start small**: Use `perPage: 10` initially, increase if needed + 2. **Fetch incrementally**: Get overview first, then details for specific items + 3. **Avoid wildcards**: Don't fetch all data when you need specific items + 4. **Use filters**: Combine `perPage` with state/label/date filters to reduce results + + ### Example Workflow Pattern + + ```markdown + # Analyze Recent Pull Requests + + 1. Fetch 10 most recent PRs (stay under token limit) + 2. For each PR, get summary without full diff + 3. If detailed analysis needed, fetch files for specific PR separately + 4. Process results incrementally rather than loading everything at once + ``` + + This proactive approach eliminates retry loops and reduces token consumption. + GH_AW_PROMPT_EOF cat << 'GH_AW_PROMPT_EOF' >> "$GH_AW_PROMPT" - {{#runtime-import .github/workflows/shared/gh.md}} + **IMPORTANT**: Always use the `safeinputs-gh` tool for GitHub CLI commands instead of running `gh` directly via bash. The `safeinputs-gh` tool has proper authentication configured with `GITHUB_TOKEN`, while bash commands do not have GitHub CLI authentication by default. + + **Correct**: + ``` + Use the safeinputs-gh tool with args: "pr list --limit 5" + Use the safeinputs-gh tool with args: "issue view 123" + ``` + + **Incorrect**: + ``` + Use the gh safe-input tool with args: "pr list --limit 5" ❌ (Wrong tool name - use safeinputs-gh) + Run: gh pr list --limit 5 ❌ (No authentication in bash) + Execute bash: gh issue view 123 ❌ (No authentication in bash) + ``` + + GH_AW_PROMPT_EOF cat << 'GH_AW_PROMPT_EOF' >> "$GH_AW_PROMPT" - {{#runtime-import .github/workflows/shared/mcp/tavily.md}} + GH_AW_PROMPT_EOF cat << 'GH_AW_PROMPT_EOF' >> "$GH_AW_PROMPT" - {{#runtime-import .github/workflows/shared/reporting.md}} + ## Report Structure Guidelines + + ### 1. Header Levels + **Use h3 (###) or lower for all headers in your issue report to maintain proper document hierarchy.** + + When creating GitHub issues or discussions: + - Use `###` (h3) for main sections (e.g., "### Test Summary") + - Use `####` (h4) for subsections (e.g., "#### Device-Specific Results") + - Never use `##` (h2) or `#` (h1) in reports - these are reserved for titles + + ### 2. Progressive Disclosure + **Wrap detailed test results in `
Section Name` tags to improve readability and reduce scrolling.** + + Use collapsible sections for: + - Verbose details (full test logs, raw data) + - Secondary information (minor warnings, extra context) + - Per-item breakdowns when there are many items + + Always keep critical information visible (summary, critical issues, key metrics). + + ### 3. Report Structure Pattern + + 1. **Overview**: 1-2 paragraphs summarizing key findings + 2. **Critical Information**: Show immediately (summary stats, critical issues) + 3. **Details**: Use `
Section Name` for expanded content + 4. **Context**: Add helpful metadata (workflow run, date, trigger) + + ### Design Principles (Airbnb-Inspired) + + Reports should: + - **Build trust through clarity**: Most important info immediately visible + - **Exceed expectations**: Add helpful context like trends, comparisons + - **Create delight**: Use progressive disclosure to reduce overwhelm + - **Maintain consistency**: Follow patterns across all reports + + ### Example Report Structure + + ```markdown + ### Summary + - Key metric 1: value + - Key metric 2: value + - Status: ✅/⚠️/❌ + + ### Critical Issues + [Always visible - these are important] + +
+ View Detailed Results + + [Comprehensive details, logs, traces] + +
+ +
+ View All Warnings + + [Minor issues and potential problems] + +
+ + ### Recommendations + [Actionable next steps - keep visible] + ``` + + ## Workflow Run References + + - Format run IDs as links: `[§12345](https://github.com/owner/repo/actions/runs/12345)` + - Include up to 3 most relevant run URLs at end under `**References:**` + - Do NOT add footer attribution (system adds automatically) GH_AW_PROMPT_EOF cat << 'GH_AW_PROMPT_EOF' >> "$GH_AW_PROMPT" - {{#runtime-import .github/workflows/shared/github-queries-safe-input.md}} + GH_AW_PROMPT_EOF cat << 'GH_AW_PROMPT_EOF' >> "$GH_AW_PROMPT" - {{#runtime-import .github/workflows/shared/go-make.md}} + **IMPORTANT**: Always use the `safeinputs-go` and `safeinputs-make` tools for Go and Make commands instead of running them directly via bash. These safe-input tools provide consistent execution and proper logging. + + **Correct**: + ``` + Use the safeinputs-go tool with args: "test ./..." + Use the safeinputs-make tool with args: "build" + Use the safeinputs-make tool with args: "lint" + Use the safeinputs-make tool with args: "test-unit" + ``` + + **Incorrect**: + ``` + Use the go safe-input tool with args: "test ./..." ❌ (Wrong tool name - use safeinputs-go) + Run: go test ./... ❌ (Use safeinputs-go instead) + Execute bash: make build ❌ (Use safeinputs-make instead) + ``` + + GH_AW_PROMPT_EOF cat << 'GH_AW_PROMPT_EOF' >> "$GH_AW_PROMPT" - {{#runtime-import .github/workflows/shared/github-mcp-app.md}} + GH_AW_PROMPT_EOF cat << 'GH_AW_PROMPT_EOF' >> "$GH_AW_PROMPT" - {{#runtime-import .github/workflows/smoke-claude.md}} + # Smoke Test: Claude Engine Validation. + + **IMPORTANT: Keep all outputs extremely short and concise. Use single-line responses where possible. No verbose explanations.** + + ## Test Requirements + + 1. **GitHub MCP Testing**: Review the last 2 merged pull requests in __GH_AW_GITHUB_REPOSITORY__ + 2. **Safe Inputs GH CLI Testing**: Use the `safeinputs-gh` tool to query 2 pull requests from __GH_AW_GITHUB_REPOSITORY__ (use args: "pr list --repo __GH_AW_GITHUB_REPOSITORY__ --limit 2 --json number,title,author") + 3. **Serena MCP Testing**: + - Use the Serena MCP server tool `activate_project` to initialize the workspace at `__GH_AW_GITHUB_WORKSPACE__` and verify it succeeds (do NOT use bash to run go commands - use Serena's MCP tools or the safeinputs-go/safeinputs-make tools from the go-make shared workflow) + - After initialization, use the `find_symbol` tool to search for symbols (find which tool to call) and verify that at least 3 symbols are found in the results + 4. **Make Build Testing**: Use the `safeinputs-make` tool to build the project (use args: "build") and verify it succeeds + 5. **Playwright Testing**: Use the playwright tools to navigate to https://github.com and verify the page title contains "GitHub" (do NOT try to install playwright - use the provided MCP tools) + 6. **Tavily Web Search Testing**: Use the Tavily MCP server to perform a web search for "GitHub Agentic Workflows" and verify that results are returned with at least one item + 7. **File Writing Testing**: Create a test file `/tmp/gh-aw/agent/smoke-test-claude-__GH_AW_GITHUB_RUN_ID__.txt` with content "Smoke test passed for Claude at $(date)" (create the directory if it doesn't exist) + 8. **Bash Tool Testing**: Execute bash commands to verify file creation was successful (use `cat` to read the file back) + 9. **Discussion Interaction Testing**: + - Use the `github-discussion-query` safe-input tool with params: `limit=1, jq=".[0]"` to get the latest discussion from __GH_AW_GITHUB_REPOSITORY__ + - Extract the discussion number from the result (e.g., if the result is `{"number": 123, "title": "...", ...}`, extract 123) + - Use the `add_comment` tool with `discussion_number: ` to add a fun, comic-book style comment stating that the smoke test agent was here + 10. **Agentic Workflows MCP Testing**: + - Call the `agentic-workflows` MCP tool using the `status` method with workflow name `smoke-claude` to query workflow status + - If the tool returns an error or no results, mark this test as ❌ and note "Tool unavailable or workflow not found" but continue to the Output section + - If the tool succeeds, extract key information from the response: total runs, success/failure counts, last run timestamp + - Write a summary of the results to `/tmp/gh-aw/agent/smoke-claude-status-__GH_AW_GITHUB_RUN_ID__.txt` (create directory if needed) + - Use bash to verify the file was created and display its contents + + ## PR Review Safe Outputs Testing + + **IMPORTANT**: The following tests require an open pull request. First, use the GitHub MCP tool to find an open PR in __GH_AW_GITHUB_REPOSITORY__ (or use the triggering PR if this is a pull_request event). Store the PR number for use in subsequent tests. + + 11. **Update PR Testing**: Use the `update_pull_request` tool to update the PR's body by appending a test message: "✨ PR Review Safe Output Test - Run __GH_AW_GITHUB_RUN_ID__" + - Use `pr_number: ` to target the open PR + - Use `operation: "append"` and `body: "\n\n---\n✨ PR Review Safe Output Test - Run __GH_AW_GITHUB_RUN_ID__"` + - Verify the tool call succeeds + + 12. **PR Review Comment Testing**: Use the `create_pull_request_review_comment` tool to add review comments on the PR + - Find a file in the PR's diff (use GitHub MCP to get PR files) + - Add at least 2 review comments on different lines with constructive feedback + - Use `pr_number: `, `path: ""`, `line: `, and `body: ""` + - Verify the tool calls succeed + + 13. **Submit PR Review Testing**: Use the `submit_pull_request_review` tool to submit a consolidated review + - Use `pr_number: `, `event: "COMMENT"`, and `body: "💥 Automated smoke test review - all systems nominal!"` + - Verify the review is submitted successfully + - Note: This will bundle all review comments from test #12 + + 14. **Resolve Review Thread Testing**: + - Use the GitHub MCP tool to list review threads on the PR + - If any threads exist, use the `resolve_pull_request_review_thread` tool to resolve one thread + - Use `thread_id: ""` from an existing thread + - If no threads exist, mark this test as ⚠️ (skipped - no threads to resolve) + + 15. **Add Reviewer Testing**: Use the `add_reviewer` tool to add a reviewer to the PR + - Use `pr_number: ` and `reviewers: ["copilot"]` (or another valid reviewer) + - Verify the tool call succeeds + - Note: May fail if reviewer is already assigned or doesn't have access + + 16. **Push to PR Branch Testing**: + - Create a test file at `/tmp/test-pr-push-__GH_AW_GITHUB_RUN_ID__.txt` with content "Test file for PR push" + - Use git commands to check if we're on the PR branch + - Use the `push_to_pull_request_branch` tool to push this change + - Use `pr_number: ` and `commit_message: "test: Add smoke test file"` + - Verify the push succeeds + - Note: This test may be skipped if not on a PR branch or if the PR is from a fork + + 17. **Close PR Testing** (CONDITIONAL - only if a test PR exists): + - If you can identify a test/bot PR that can be safely closed, use the `close_pull_request` tool + - Use `pr_number: ` and `comment: "Closing as part of smoke test - Run __GH_AW_GITHUB_RUN_ID__"` + - If no suitable test PR exists, mark this test as ⚠️ (skipped - no safe PR to close) + - **DO NOT close the triggering PR or any important PRs** + + ## Output + + **CRITICAL: You MUST create an issue regardless of test results - this is a required safe output.** + + 1. **ALWAYS create an issue** with a summary of the smoke test run: + - Title: "Smoke Test: Claude - __GH_AW_GITHUB_RUN_ID__" + - Body should include: + - Test results (✅ for pass, ❌ for fail, ⚠️ for skipped) for each test (including PR review tests #11-17) + - Overall status: PASS (all passed), PARTIAL (some skipped), or FAIL (any failed) + - Run URL: __GH_AW_GITHUB_SERVER_URL__/__GH_AW_GITHUB_REPOSITORY__/actions/runs/__GH_AW_GITHUB_RUN_ID__ + - Timestamp + - Note which PR was used for PR review testing (if applicable) + - If ANY test fails, include error details in the issue body + - This issue MUST be created before any other safe output operations + + 2. **Only if this workflow was triggered by a pull_request event**: Use the `add_comment` tool to add a **very brief** comment (max 5-10 lines) to the triggering pull request (omit the `item_number` parameter to auto-target the triggering PR) with: + - Test results for core tests #1-10 (✅ or ❌) + - Test results for PR review tests #11-17 (✅, ❌, or ⚠️) + - Overall status: PASS, PARTIAL, or FAIL + + 3. Use the `add_comment` tool with `item_number` set to the discussion number you extracted in step 9 to add a **fun comic-book style comment** to that discussion - be playful and use comic-book language like "💥 WHOOSH!" + - If step 9 failed to extract a discussion number, skip this step + + If all non-skipped tests pass, use the `add_labels` tool to add the label `smoke-claude` to the pull request (omit the `item_number` parameter to auto-target the triggering PR if this workflow was triggered by a pull_request event). + GH_AW_PROMPT_EOF - name: Interpolate variables and render templates uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 diff --git a/.github/workflows/smoke-claude.md b/.github/workflows/smoke-claude.md index 798617ddf2..baf75aa6d2 100644 --- a/.github/workflows/smoke-claude.md +++ b/.github/workflows/smoke-claude.md @@ -20,6 +20,7 @@ engine: id: claude max-turns: 100 strict: true +inlined-imports: true imports: - shared/mcp-pagination.md - shared/gh.md diff --git a/pkg/parser/frontmatter_hash.go b/pkg/parser/frontmatter_hash.go index 111d5c16fc..71e1d64031 100644 --- a/pkg/parser/frontmatter_hash.go +++ b/pkg/parser/frontmatter_hash.go @@ -17,6 +17,19 @@ import ( var frontmatterHashLog = logger.New("parser:frontmatter_hash") +// parseBoolFromFrontmatter extracts a boolean value from a frontmatter map. +// Returns false if the key is absent, the map is nil, or the value is not a bool. +func parseBoolFromFrontmatter(m map[string]any, key string) bool { + if m == nil { + return false + } + if v, ok := m[key]; ok { + b, _ := v.(bool) + return b + } + return false +} + // FileReader is a function type that reads file content // This abstraction allows for different file reading strategies (disk, GitHub API, in-memory, etc.) type FileReader func(filePath string) ([]byte, error) @@ -261,8 +274,24 @@ func ComputeFrontmatterHashFromFile(filePath string, cache *ImportCache) (string return ComputeFrontmatterHashFromFileWithReader(filePath, cache, DefaultFileReader) } +// ComputeFrontmatterHashFromFileWithParsedFrontmatter computes the frontmatter hash using +// a pre-parsed frontmatter map. The parsedFrontmatter must not be nil; callers are responsible +// for parsing the frontmatter before calling this function. +func ComputeFrontmatterHashFromFileWithParsedFrontmatter(filePath string, parsedFrontmatter map[string]any, cache *ImportCache, fileReader FileReader) (string, error) { + frontmatterHashLog.Printf("Computing hash for file: %s", filePath) + + // Read file content using the provided file reader + content, err := fileReader(filePath) + if err != nil { + return "", fmt.Errorf("failed to read file: %w", err) + } + + return computeFrontmatterHashFromContent(string(content), parsedFrontmatter, filePath, cache, fileReader) +} + // ComputeFrontmatterHashFromFileWithReader computes the frontmatter hash for a workflow file // using a custom file reader function (e.g., for GitHub API, in-memory file system, etc.) +// It parses the frontmatter once from the file content, then delegates to the core logic. func ComputeFrontmatterHashFromFileWithReader(filePath string, cache *ImportCache, fileReader FileReader) (string, error) { frontmatterHashLog.Printf("Computing hash for file: %s", filePath) @@ -272,8 +301,20 @@ func ComputeFrontmatterHashFromFileWithReader(filePath string, cache *ImportCach return "", fmt.Errorf("failed to read file: %w", err) } + // Parse frontmatter once from content; treat inlined-imports as false if parsing fails + var parsedFrontmatter map[string]any + if parsed, parseErr := ExtractFrontmatterFromContent(string(content)); parseErr == nil { + parsedFrontmatter = parsed.Frontmatter + } + + return computeFrontmatterHashFromContent(string(content), parsedFrontmatter, filePath, cache, fileReader) +} + +// computeFrontmatterHashFromContent is the shared core that computes the hash given the +// already-read file content and pre-parsed frontmatter map (may be nil). +func computeFrontmatterHashFromContent(content string, parsedFrontmatter map[string]any, filePath string, cache *ImportCache, fileReader FileReader) (string, error) { // Extract frontmatter and markdown as text (no YAML parsing) - frontmatterText, markdown, err := extractFrontmatterAndBodyText(string(content)) + frontmatterText, markdown, err := extractFrontmatterAndBodyText(content) if err != nil { return "", fmt.Errorf("failed to extract frontmatter: %w", err) } @@ -281,11 +322,23 @@ func ComputeFrontmatterHashFromFileWithReader(filePath string, cache *ImportCach // Get base directory for resolving imports baseDir := filepath.Dir(filePath) - // Extract relevant template expressions from markdown body - relevantExpressions := extractRelevantTemplateExpressions(markdown) + // Detect inlined-imports from the pre-parsed frontmatter map. + // If nil (parsing failed or not provided), inlined-imports is treated as false. + inlinedImports := parseBoolFromFrontmatter(parsedFrontmatter, "inlined-imports") + + // When inlined-imports is enabled, the entire markdown body is compiled into the lock + // file, so any change to the body must invalidate the hash. Include the full body text. + // Otherwise, only extract the relevant template expressions (env./vars. references). + var relevantExpressions []string + var fullBody string + if inlinedImports { + fullBody = normalizeFrontmatterText(markdown) + } else { + relevantExpressions = extractRelevantTemplateExpressions(markdown) + } // Compute hash using text-based approach with custom file reader - return computeFrontmatterHashTextBasedWithReader(frontmatterText, markdown, baseDir, cache, relevantExpressions, fileReader) + return computeFrontmatterHashTextBasedWithReader(frontmatterText, fullBody, baseDir, cache, relevantExpressions, fileReader) } // ComputeFrontmatterHashWithExpressions computes the hash including template expressions @@ -517,7 +570,9 @@ func processImportsTextBased(frontmatterText, baseDir string, visited map[string return importedFiles, importedFrontmatterTexts, nil } -// computeFrontmatterHashTextBasedWithReader computes the hash using text-based approach with custom file reader +// computeFrontmatterHashTextBasedWithReader computes the hash using text-based approach with custom file reader. +// When markdown is non-empty, it is included as the full body text in the canonical data (used for +// inlined-imports mode where the entire body is compiled into the lock file). func computeFrontmatterHashTextBasedWithReader(frontmatterText, markdown, baseDir string, cache *ImportCache, expressions []string, fileReader FileReader) (string, error) { frontmatterHashLog.Print("Computing frontmatter hash using text-based approach") @@ -553,8 +608,11 @@ func computeFrontmatterHashTextBasedWithReader(frontmatterText, markdown, baseDi canonical["imported-frontmatters"] = strings.Join(normalizedTexts, "\n---\n") } - // Add template expressions if present - if len(expressions) > 0 { + // When inlined-imports is enabled, include the full markdown body so any content + // change invalidates the hash. Otherwise, include only relevant template expressions. + if markdown != "" { + canonical["body-text"] = markdown + } else if len(expressions) > 0 { canonical["template-expressions"] = expressions } diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 86ead0ca20..07e87f64bf 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -120,6 +120,12 @@ ] ] }, + "inlined-imports": { + "type": "boolean", + "default": false, + "description": "If true, inline all imports (including those without inputs) at compilation time in the generated lock.yml instead of using runtime-import macros. When enabled, the frontmatter hash covers the entire markdown body so any change to the content will invalidate the hash.", + "examples": [true, false] + }, "on": { "description": "Workflow triggers that define when the agentic workflow should run. Supports standard GitHub Actions trigger events plus special command triggers for /commands (required)", "examples": [ diff --git a/pkg/workflow/compiler_orchestrator_workflow.go b/pkg/workflow/compiler_orchestrator_workflow.go index c816158332..c294940034 100644 --- a/pkg/workflow/compiler_orchestrator_workflow.go +++ b/pkg/workflow/compiler_orchestrator_workflow.go @@ -118,6 +118,17 @@ func (c *Compiler) buildInitialWorkflowData( ) *WorkflowData { orchestratorWorkflowLog.Print("Building initial workflow data") + inlinedImports := resolveInlinedImports(result.Frontmatter) + + // When inlined-imports is true, agent file content is already inlined via ImportPaths → step 1b. + // Clear AgentFile/AgentImportSpec so engines don't read it from disk separately at runtime. + agentFile := importsResult.AgentFile + agentImportSpec := importsResult.AgentImportSpec + if inlinedImports { + agentFile = "" + agentImportSpec = "" + } + return &WorkflowData{ Name: toolsResult.workflowName, FrontmatterName: toolsResult.frontmatterName, @@ -138,8 +149,8 @@ func (c *Compiler) buildInitialWorkflowData( MarkdownContent: toolsResult.markdownContent, AI: engineSetup.engineSetting, EngineConfig: engineSetup.engineConfig, - AgentFile: importsResult.AgentFile, - AgentImportSpec: importsResult.AgentImportSpec, + AgentFile: agentFile, + AgentImportSpec: agentImportSpec, RepositoryImports: importsResult.RepositoryImports, NetworkPermissions: engineSetup.networkPermissions, SandboxConfig: applySandboxDefaults(engineSetup.sandboxConfig, engineSetup.engineConfig), @@ -151,11 +162,20 @@ func (c *Compiler) buildInitialWorkflowData( StrictMode: c.strictMode, SecretMasking: toolsResult.secretMasking, ParsedFrontmatter: toolsResult.parsedFrontmatter, + RawFrontmatter: result.Frontmatter, HasExplicitGitHubTool: toolsResult.hasExplicitGitHubTool, ActionMode: c.actionMode, + InlinedImports: inlinedImports, } } +// resolveInlinedImports returns true if inlined-imports is enabled. +// It reads the value directly from the raw (pre-parsed) frontmatter map, which is always +// populated regardless of whether ParseFrontmatterConfig succeeded. +func resolveInlinedImports(rawFrontmatter map[string]any) bool { + return ParseBoolFromConfig(rawFrontmatter, "inlined-imports", nil) +} + // extractYAMLSections extracts YAML configuration sections from frontmatter func (c *Compiler) extractYAMLSections(frontmatter map[string]any, workflowData *WorkflowData) { orchestratorWorkflowLog.Print("Extracting YAML sections from frontmatter") diff --git a/pkg/workflow/compiler_types.go b/pkg/workflow/compiler_types.go index 83277a0690..8af4660d9d 100644 --- a/pkg/workflow/compiler_types.go +++ b/pkg/workflow/compiler_types.go @@ -457,9 +457,11 @@ type WorkflowData struct { StrictMode bool // strict mode for action pinning SecretMasking *SecretMaskingConfig // secret masking configuration ParsedFrontmatter *FrontmatterConfig // cached parsed frontmatter configuration (for performance optimization) + RawFrontmatter map[string]any // raw parsed frontmatter map (for passing to hash functions without re-parsing) ActionPinWarnings map[string]bool // cache of already-warned action pin failures (key: "repo@version") ActionMode ActionMode // action mode for workflow compilation (dev, release, script) HasExplicitGitHubTool bool // true if tools.github was explicitly configured in frontmatter + InlinedImports bool // if true, inline all imports at compile time (from inlined-imports frontmatter field) } // BaseSafeOutputConfig holds common configuration fields for all safe output types diff --git a/pkg/workflow/compiler_yaml.go b/pkg/workflow/compiler_yaml.go index 19ce4fffc3..26553ca6a1 100644 --- a/pkg/workflow/compiler_yaml.go +++ b/pkg/workflow/compiler_yaml.go @@ -3,6 +3,7 @@ package workflow import ( "encoding/json" "fmt" + "os" "path/filepath" "sort" "strings" @@ -107,6 +108,12 @@ func (c *Compiler) generateWorkflowHeader(yaml *strings.Builder, data *WorkflowD } } + // Add inlined-imports comment to indicate the field was used at compile time + if data.InlinedImports { + yaml.WriteString("#\n") + yaml.WriteString("# inlined-imports: true\n") + } + // Add lock metadata (schema version + frontmatter hash + stop time) as JSON // Single-line format to minimize merge conflicts and be unaffected by LOC changes if frontmatterHash != "" { @@ -182,7 +189,7 @@ func (c *Compiler) generateYAML(data *WorkflowData, markdownPath string) (string if markdownPath != "" { baseDir := filepath.Dir(markdownPath) cache := parser.NewImportCache(baseDir) - hash, err := parser.ComputeFrontmatterHashFromFile(markdownPath, cache) + hash, err := parser.ComputeFrontmatterHashFromFileWithParsedFrontmatter(markdownPath, data.RawFrontmatter, cache, parser.DefaultFileReader) if err != nil { compilerYamlLog.Printf("Warning: failed to compute frontmatter hash: %v", err) // Continue without hash - non-fatal error @@ -268,42 +275,52 @@ func (c *Compiler) generatePrompt(yaml *strings.Builder, data *WorkflowData) { if data.ImportedMarkdown != "" { compilerYamlLog.Printf("Processing imported markdown (%d bytes)", len(data.ImportedMarkdown)) - // Clean and process imported markdown - cleanedImportedMarkdown := removeXMLComments(data.ImportedMarkdown) - - // Substitute import inputs in imported content + // Clean, substitute, and post-process imported markdown + cleaned := removeXMLComments(data.ImportedMarkdown) if len(data.ImportInputs) > 0 { compilerYamlLog.Printf("Substituting %d import input values", len(data.ImportInputs)) - cleanedImportedMarkdown = SubstituteImportInputs(cleanedImportedMarkdown, data.ImportInputs) - } - - // Wrap GitHub expressions in template conditionals - cleanedImportedMarkdown = wrapExpressionsInTemplateConditionals(cleanedImportedMarkdown) - - // Extract expressions from imported content - extractor := NewExpressionExtractor() - importedExprMappings, err := extractor.ExtractExpressions(cleanedImportedMarkdown) - if err == nil && len(importedExprMappings) > 0 { - cleanedImportedMarkdown = extractor.ReplaceExpressionsWithEnvVars(cleanedImportedMarkdown) - expressionMappings = importedExprMappings + cleaned = SubstituteImportInputs(cleaned, data.ImportInputs) } - - // Split imported content into chunks and add to user prompt - importedChunks := splitContentIntoChunks(cleanedImportedMarkdown) - userPromptChunks = append(userPromptChunks, importedChunks...) - compilerYamlLog.Printf("Inlined imported markdown with inputs in %d chunks", len(importedChunks)) + chunks, exprMaps := processMarkdownBody(cleaned) + userPromptChunks = append(userPromptChunks, chunks...) + expressionMappings = exprMaps + compilerYamlLog.Printf("Inlined imported markdown with inputs in %d chunks", len(chunks)) } - // Step 1b: Generate runtime-import macros for imported markdown without inputs - // These imports don't need compile-time substitution, so they can be loaded at runtime + // Step 1b: For imports without inputs: + // - inlinedImports mode (inlined-imports: true frontmatter): read and inline content at compile time + // - normal mode: generate runtime-import macros (loaded at runtime) if len(data.ImportPaths) > 0 { - compilerYamlLog.Printf("Generating runtime-import macros for %d imports without inputs", len(data.ImportPaths)) - for _, importPath := range data.ImportPaths { - // Normalize to Unix paths (forward slashes) for cross-platform compatibility - importPath = filepath.ToSlash(importPath) - runtimeImportMacro := fmt.Sprintf("{{#runtime-import %s}}", importPath) - userPromptChunks = append(userPromptChunks, runtimeImportMacro) - compilerYamlLog.Printf("Added runtime-import macro for: %s", importPath) + if data.InlinedImports && c.markdownPath != "" { + // inlinedImports mode: read import file content from disk and embed directly + compilerYamlLog.Printf("Inlining %d imports without inputs at compile time", len(data.ImportPaths)) + workspaceRoot := resolveWorkspaceRoot(c.markdownPath) + for _, importPath := range data.ImportPaths { + importPath = filepath.ToSlash(importPath) + rawContent, err := os.ReadFile(filepath.Join(workspaceRoot, importPath)) + if err != nil { + // Fall back to runtime-import macro if file cannot be read + compilerYamlLog.Printf("Warning: failed to read import file %s (%v), falling back to runtime-import", importPath, err) + userPromptChunks = append(userPromptChunks, fmt.Sprintf("{{#runtime-import %s}}", importPath)) + continue + } + importedBody, extractErr := parser.ExtractMarkdownContent(string(rawContent)) + if extractErr != nil { + importedBody = string(rawContent) + } + chunks, exprMaps := processMarkdownBody(importedBody) + userPromptChunks = append(userPromptChunks, chunks...) + expressionMappings = append(expressionMappings, exprMaps...) + compilerYamlLog.Printf("Inlined import without inputs: %s", importPath) + } + } else { + // Normal mode: generate runtime-import macros (loaded at workflow runtime) + compilerYamlLog.Printf("Generating runtime-import macros for %d imports without inputs", len(data.ImportPaths)) + for _, importPath := range data.ImportPaths { + importPath = filepath.ToSlash(importPath) + userPromptChunks = append(userPromptChunks, fmt.Sprintf("{{#runtime-import %s}}", importPath)) + compilerYamlLog.Printf("Added runtime-import macro for: %s", importPath) + } } } @@ -313,7 +330,7 @@ func (c *Compiler) generatePrompt(yaml *strings.Builder, data *WorkflowData) { // available at compile time for the substitute placeholders step // Use MainWorkflowMarkdown (not MarkdownContent) to avoid extracting from imported content // Skip this step when inlinePrompt is true because expression extraction happens in Step 2 - if !c.inlinePrompt && data.MainWorkflowMarkdown != "" { + if !c.inlinePrompt && !data.InlinedImports && data.MainWorkflowMarkdown != "" { compilerYamlLog.Printf("Extracting expressions from main workflow markdown (%d bytes)", len(data.MainWorkflowMarkdown)) // Create a new extractor for main workflow markdown @@ -327,7 +344,7 @@ func (c *Compiler) generatePrompt(yaml *strings.Builder, data *WorkflowData) { } // Step 2: Add main workflow markdown content to the prompt - if c.inlinePrompt { + if c.inlinePrompt || data.InlinedImports { // Inline mode (Wasm/browser): embed the markdown content directly in the YAML // since runtime-import macros cannot resolve without filesystem access if data.MainWorkflowMarkdown != "" { @@ -712,3 +729,36 @@ func (c *Compiler) generateOutputCollectionStep(yaml *strings.Builder, data *Wor yaml.WriteString(" if-no-files-found: warn\n") } + +// processMarkdownBody applies the standard post-processing pipeline to a markdown body: +// XML comment removal, expression wrapping, expression extraction/substitution, and chunking. +// It returns the prompt chunks and expression mappings extracted from the content. +func processMarkdownBody(body string) ([]string, []*ExpressionMapping) { + body = removeXMLComments(body) + body = wrapExpressionsInTemplateConditionals(body) + extractor := NewExpressionExtractor() + exprMappings, err := extractor.ExtractExpressions(body) + if err == nil && len(exprMappings) > 0 { + body = extractor.ReplaceExpressionsWithEnvVars(body) + } else { + exprMappings = nil + } + return splitContentIntoChunks(body), exprMappings +} + +// resolveWorkspaceRoot returns the workspace root directory given the path to a workflow markdown +// file. ImportPaths are relative to the workspace root (e.g. ".github/workflows/shared/foo.md"), +// so the workspace root is the directory that contains ".github/". +func resolveWorkspaceRoot(markdownPath string) string { + normalized := filepath.ToSlash(markdownPath) + if idx := strings.Index(normalized, "/.github/"); idx != -1 { + // Absolute or non-root-relative path: strip everything from "/.github/" onward. + return filepath.FromSlash(normalized[:idx]) + } + if strings.HasPrefix(normalized, ".github/") { + // Path already starts at the workspace root. + return "." + } + // Fallback: use the directory containing the workflow file. + return filepath.Dir(markdownPath) +} diff --git a/pkg/workflow/frontmatter_types.go b/pkg/workflow/frontmatter_types.go index 41ed80ed57..9791c764f7 100644 --- a/pkg/workflow/frontmatter_types.go +++ b/pkg/workflow/frontmatter_types.go @@ -144,8 +144,9 @@ type FrontmatterConfig struct { Cache map[string]any `json:"cache,omitempty"` // Import and inclusion - Imports any `json:"imports,omitempty"` // Can be string or array - Include any `json:"include,omitempty"` // Can be string or array + Imports any `json:"imports,omitempty"` // Can be string or array + Include any `json:"include,omitempty"` // Can be string or array + InlinedImports bool `json:"inlined-imports,omitempty"` // If true, inline all imports at compile time instead of using runtime-import macros // Metadata Metadata map[string]string `json:"metadata,omitempty"` // Custom metadata key-value pairs diff --git a/pkg/workflow/inline_imports_test.go b/pkg/workflow/inline_imports_test.go new file mode 100644 index 0000000000..ce737d510f --- /dev/null +++ b/pkg/workflow/inline_imports_test.go @@ -0,0 +1,343 @@ +//go:build !integration + +package workflow + +import ( + "os" + "path/filepath" + "testing" + + "github.com/github/gh-aw/pkg/parser" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestInlinedImports_FrontmatterField verifies that inlined-imports: true activates +// compile-time inlining of imports (without inputs) and the main workflow markdown. +func TestInlinedImports_FrontmatterField(t *testing.T) { + tmpDir := t.TempDir() + + // Create a shared import file with markdown content + sharedDir := filepath.Join(tmpDir, ".github", "workflows", "shared") + require.NoError(t, os.MkdirAll(sharedDir, 0o755)) + sharedFile := filepath.Join(sharedDir, "common.md") + sharedContent := `--- +tools: + bash: true +--- + +# Shared Instructions + +Always follow best practices. +` + require.NoError(t, os.WriteFile(sharedFile, []byte(sharedContent), 0o644)) + + // Create the main workflow file with inlined-imports: true + workflowDir := filepath.Join(tmpDir, ".github", "workflows") + workflowFile := filepath.Join(workflowDir, "test-workflow.md") + workflowContent := `--- +name: inlined-imports-test +on: + workflow_dispatch: +permissions: + contents: read +engine: copilot +inlined-imports: true +imports: + - shared/common.md +--- + +# Main Workflow + +This is the main workflow content. +` + require.NoError(t, os.WriteFile(workflowFile, []byte(workflowContent), 0o644)) + + compiler := NewCompiler( + WithNoEmit(true), + WithSkipValidation(true), + ) + + wd, err := compiler.ParseWorkflowFile(workflowFile) + require.NoError(t, err, "should parse workflow file") + require.NotNil(t, wd) + + // WorkflowData.InlinedImports should be true (parsed into the workspace data) + assert.True(t, wd.InlinedImports, "WorkflowData.InlinedImports should be true") + + // ParsedFrontmatter should also have InlinedImports = true + require.NotNil(t, wd.ParsedFrontmatter, "ParsedFrontmatter should not be nil") + assert.True(t, wd.ParsedFrontmatter.InlinedImports, "InlinedImports should be true") + + // Compile and get YAML + yamlContent, err := compiler.CompileToYAML(wd, workflowFile) + require.NoError(t, err, "should compile workflow") + require.NotEmpty(t, yamlContent, "YAML should not be empty") + + // With inlined-imports: true, the import should be inlined (no runtime-import macros) + assert.NotContains(t, yamlContent, "{{#runtime-import", "should not generate any runtime-import macros") + + // The shared content should be inlined in the prompt + assert.Contains(t, yamlContent, "Shared Instructions", "shared import content should be inlined") + assert.Contains(t, yamlContent, "Always follow best practices", "shared import content should be inlined") + + // The main workflow content should also be inlined (no runtime-import for main file) + assert.Contains(t, yamlContent, "Main Workflow", "main workflow content should be inlined") + assert.Contains(t, yamlContent, "This is the main workflow content", "main workflow content should be inlined") +} + +// TestInlinedImports_Disabled verifies that without inlined-imports, runtime-import macros are used. +func TestInlinedImports_Disabled(t *testing.T) { + tmpDir := t.TempDir() + + sharedDir := filepath.Join(tmpDir, ".github", "workflows", "shared") + require.NoError(t, os.MkdirAll(sharedDir, 0o755)) + sharedFile := filepath.Join(sharedDir, "common.md") + sharedContent := `--- +tools: + bash: true +--- + +# Shared Instructions + +Always follow best practices. +` + require.NoError(t, os.WriteFile(sharedFile, []byte(sharedContent), 0o644)) + + workflowDir := filepath.Join(tmpDir, ".github", "workflows") + workflowFile := filepath.Join(workflowDir, "test-workflow.md") + workflowContent := `--- +name: no-inlined-imports-test +on: + workflow_dispatch: +permissions: + contents: read +engine: copilot +imports: + - shared/common.md +--- + +# Main Workflow + +This is the main workflow content. +` + require.NoError(t, os.WriteFile(workflowFile, []byte(workflowContent), 0o644)) + + compiler := NewCompiler( + WithNoEmit(true), + WithSkipValidation(true), + ) + + wd, err := compiler.ParseWorkflowFile(workflowFile) + require.NoError(t, err, "should parse workflow file") + require.NotNil(t, wd) + + require.NotNil(t, wd.ParsedFrontmatter, "ParsedFrontmatter should be populated") + assert.False(t, wd.ParsedFrontmatter.InlinedImports, "InlinedImports should be false by default") + + yamlContent, err := compiler.CompileToYAML(wd, workflowFile) + require.NoError(t, err, "should compile workflow") + + // Without inlined-imports, the import should use runtime-import macro (with full path from workspace root) + assert.Contains(t, yamlContent, "{{#runtime-import .github/workflows/shared/common.md}}", "should generate runtime-import macro for import") + + // The main workflow markdown should also use a runtime-import macro + assert.Contains(t, yamlContent, "{{#runtime-import .github/workflows/test-workflow.md}}", "should generate runtime-import macro for main workflow") +} + +// TestInlinedImports_HashChangesWithBody verifies that the frontmatter hash includes +// the entire markdown body when inlined-imports: true. +func TestInlinedImports_HashChangesWithBody(t *testing.T) { + tmpDir := t.TempDir() + + content1 := `--- +name: test +on: + workflow_dispatch: +inlined-imports: true +engine: copilot +--- + +# Original body +` + content2 := `--- +name: test +on: + workflow_dispatch: +inlined-imports: true +engine: copilot +--- + +# Modified body - different +` + // Normal mode (no inlined-imports) - body changes should not affect hash + contentNormal1 := `--- +name: test +on: + workflow_dispatch: +engine: copilot +--- + +# Body variant A +` + contentNormal2 := `--- +name: test +on: + workflow_dispatch: +engine: copilot +--- + +# Body variant B - same hash expected +` + + file1 := filepath.Join(tmpDir, "test1.md") + file2 := filepath.Join(tmpDir, "test2.md") + fileN1 := filepath.Join(tmpDir, "normal1.md") + fileN2 := filepath.Join(tmpDir, "normal2.md") + require.NoError(t, os.WriteFile(file1, []byte(content1), 0o644)) + require.NoError(t, os.WriteFile(file2, []byte(content2), 0o644)) + require.NoError(t, os.WriteFile(fileN1, []byte(contentNormal1), 0o644)) + require.NoError(t, os.WriteFile(fileN2, []byte(contentNormal2), 0o644)) + + cache := parser.NewImportCache(tmpDir) + + hash1, err := parser.ComputeFrontmatterHashFromFile(file1, cache) + require.NoError(t, err) + hash2, err := parser.ComputeFrontmatterHashFromFile(file2, cache) + require.NoError(t, err) + hashN1, err := parser.ComputeFrontmatterHashFromFile(fileN1, cache) + require.NoError(t, err) + hashN2, err := parser.ComputeFrontmatterHashFromFile(fileN2, cache) + require.NoError(t, err) + + // With inlined-imports: true, different body content should produce different hashes + assert.NotEqual(t, hash1, hash2, + "with inlined-imports: true, different body content should produce different hashes") + + // Without inlined-imports, body-only changes produce the same hash + // (only env./vars. expressions from body are included) + assert.Equal(t, hashN1, hashN2, + "without inlined-imports, body-only changes should not affect hash") + + // inlined-imports mode should also produce a different hash than normal mode + // (frontmatter text differs, so hash differs regardless of body treatment) + assert.NotEqual(t, hash1, hashN1, + "inlined-imports and normal mode should produce different hashes (different frontmatter)") +} + +// TestInlinedImports_FrontmatterHashInline_SameBodySameHash verifies determinism. +func TestInlinedImports_FrontmatterHashInline_SameBodySameHash(t *testing.T) { + tmpDir := t.TempDir() + content := `--- +name: test +on: + workflow_dispatch: +inlined-imports: true +engine: copilot +--- + +# Same body content +` + file1 := filepath.Join(tmpDir, "a.md") + file2 := filepath.Join(tmpDir, "b.md") + require.NoError(t, os.WriteFile(file1, []byte(content), 0o644)) + require.NoError(t, os.WriteFile(file2, []byte(content), 0o644)) + + cache := parser.NewImportCache(tmpDir) + hash1, err := parser.ComputeFrontmatterHashFromFile(file1, cache) + require.NoError(t, err) + hash2, err := parser.ComputeFrontmatterHashFromFile(file2, cache) + require.NoError(t, err) + + assert.Equal(t, hash1, hash2, "same content should produce the same hash") +} + +// TestInlinedImports_InlinePromptActivated verifies that inlined-imports also activates inline prompt mode. +func TestInlinedImports_InlinePromptActivated(t *testing.T) { + tmpDir := t.TempDir() + + workflowDir := filepath.Join(tmpDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowDir, 0o755)) + workflowFile := filepath.Join(workflowDir, "inline-test.md") + workflowContent := `--- +name: inline-test +on: + workflow_dispatch: +permissions: + contents: read +engine: copilot +inlined-imports: true +--- + +# My Workflow + +Do something useful. +` + require.NoError(t, os.WriteFile(workflowFile, []byte(workflowContent), 0o644)) + + compiler := NewCompiler( + WithNoEmit(true), + WithSkipValidation(true), + ) + + wd, err := compiler.ParseWorkflowFile(workflowFile) + require.NoError(t, err) + + yamlContent, err := compiler.CompileToYAML(wd, workflowFile) + require.NoError(t, err) + + // When inlined-imports is true, the main markdown body is also inlined (no runtime-import for main file) + assert.NotContains(t, yamlContent, "{{#runtime-import", "should not generate any runtime-import macros") + // Main workflow content should be inlined + assert.Contains(t, yamlContent, "My Workflow", "main workflow content should be inlined") + assert.Contains(t, yamlContent, "Do something useful", "main workflow body should be inlined") +} + +// TestInlinedImports_AgentFileCleared verifies that when inlined-imports: true, the AgentFile +// field is cleared in WorkflowData so the engine doesn't read it from disk separately +// (the agent content is already inlined via ImportPaths → step 1b). +func TestInlinedImports_AgentFileCleared(t *testing.T) { + compiler := NewCompiler() + + frontmatterResult := &parser.FrontmatterResult{ + Frontmatter: map[string]any{ + "name": "agent-test", + "engine": "copilot", + "inlined-imports": true, + }, + FrontmatterLines: []string{ + "name: agent-test", + "engine: copilot", + "inlined-imports: true", + }, + } + + toolsResult := &toolsProcessingResult{ + workflowName: "agent-test", + frontmatterName: "agent-test", + parsedFrontmatter: &FrontmatterConfig{Name: "agent-test", Engine: "copilot", InlinedImports: true}, + tools: map[string]any{}, + importPaths: []string{".github/agents/my-agent.md"}, + mainWorkflowMarkdown: "# Main", + } + + engineSetup := &engineSetupResult{ + engineSetting: "copilot", + engineConfig: &EngineConfig{ID: "copilot"}, + sandboxConfig: &SandboxConfig{}, + } + + importsResult := &parser.ImportsResult{ + AgentFile: ".github/agents/my-agent.md", + AgentImportSpec: ".github/agents/my-agent.md", + } + + wd := compiler.buildInitialWorkflowData(frontmatterResult, toolsResult, engineSetup, importsResult) + + // InlinedImports should be true in WorkflowData + assert.True(t, wd.InlinedImports, "InlinedImports should be true in WorkflowData") + + // AgentFile should be cleared (content inlined via ImportPaths instead) + assert.Empty(t, wd.AgentFile, "AgentFile should be cleared when inlined-imports is true") + assert.Empty(t, wd.AgentImportSpec, "AgentImportSpec should be cleared when inlined-imports is true") +} diff --git a/test-pr-push-22206642065.txt b/test-pr-push-22206642065.txt new file mode 100644 index 0000000000..545d3c5857 --- /dev/null +++ b/test-pr-push-22206642065.txt @@ -0,0 +1 @@ +Test file for PR push