diff --git a/actions/setup/js/safe_output_unified_handler_manager.cjs b/actions/setup/js/safe_output_unified_handler_manager.cjs index 2ffd19621b..1d2b79fec9 100644 --- a/actions/setup/js/safe_output_unified_handler_manager.cjs +++ b/actions/setup/js/safe_output_unified_handler_manager.cjs @@ -153,6 +153,7 @@ const PROJECT_RELATED_TYPES = new Set(Object.keys(PROJECT_HANDLER_MAP)); /** * Load configuration for safe outputs * Reads configuration from both GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG and GH_AW_SAFE_OUTPUTS_PROJECT_HANDLER_CONFIG + * Automatically splits project handlers from regular config if they're in the wrong place * @returns {{regular: Object, project: Object}} Safe outputs configuration for regular and project handlers */ function loadConfig() { @@ -163,20 +164,36 @@ function loadConfig() { if (process.env.GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG) { try { const config = JSON.parse(process.env.GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG); - core.info(`Loaded regular handler config: ${JSON.stringify(config)}`); + core.info(`Loaded config from GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: ${JSON.stringify(config)}`); + // Normalize config keys: convert hyphens to underscores - Object.assign(regular, Object.fromEntries(Object.entries(config).map(([k, v]) => [k.replace(/-/g, "_"), v]))); + const normalizedEntries = Object.entries(config).map(([k, v]) => [k.replace(/-/g, "_"), v]); + + // Automatically split project handlers from regular handlers + // Project handlers (update_project, create_project, create_project_status_update) require + // a separate Octokit client authenticated with GH_AW_PROJECT_GITHUB_TOKEN because they need + // Projects permissions that differ from regular handler permissions. This auto-split ensures + // backward compatibility with the Go compiler which puts all handlers in a unified config. + for (const [key, value] of normalizedEntries) { + if (PROJECT_RELATED_TYPES.has(key)) { + project[key] = value; + core.info(`Auto-moved ${key} from unified config to project config (requires project token)`); + } else { + regular[key] = value; + } + } } catch (error) { throw new Error(`Failed to parse GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: ${getErrorMessage(error)}`); } } - // Load project handler config + // Load project handler config (if explicitly provided, merge with auto-split handlers) if (process.env.GH_AW_SAFE_OUTPUTS_PROJECT_HANDLER_CONFIG) { try { const config = JSON.parse(process.env.GH_AW_SAFE_OUTPUTS_PROJECT_HANDLER_CONFIG); core.info(`Loaded project handler config: ${JSON.stringify(config)}`); // Normalize config keys: convert hyphens to underscores + // Explicitly provided project config takes precedence over auto-split config Object.assign(project, Object.fromEntries(Object.entries(config).map(([k, v]) => [k.replace(/-/g, "_"), v]))); } catch (error) { throw new Error(`Failed to parse GH_AW_SAFE_OUTPUTS_PROJECT_HANDLER_CONFIG: ${getErrorMessage(error)}`); @@ -188,6 +205,13 @@ function loadConfig() { throw new Error("At least one of GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG or GH_AW_SAFE_OUTPUTS_PROJECT_HANDLER_CONFIG environment variables is required"); } + const regularCount = Object.keys(regular).length; + const projectCount = Object.keys(project).length; + core.info(`Configuration loaded: ${regularCount} regular handler${regularCount === 1 ? "" : "s"}, ${projectCount} project handler${projectCount === 1 ? "" : "s"}`); + if (projectCount > 0) { + core.info(`Project handlers: ${Object.keys(project).join(", ")}`); + } + return { regular, project }; } diff --git a/actions/setup/js/safe_output_unified_handler_manager.test.cjs b/actions/setup/js/safe_output_unified_handler_manager.test.cjs index 28f05a0a0e..317652ca9a 100644 --- a/actions/setup/js/safe_output_unified_handler_manager.test.cjs +++ b/actions/setup/js/safe_output_unified_handler_manager.test.cjs @@ -97,6 +97,116 @@ describe("Unified Safe Output Handler Manager", () => { expect(config.regular).toHaveProperty("create_issue"); expect(config.regular).not.toHaveProperty("create-issue"); }); + + it("should automatically split project handlers from unified config", () => { + // Simulate Go compiler putting all handlers in one config + process.env.GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG = JSON.stringify({ + create_issue: { max: 5 }, + add_comment: {}, + update_project: { max: 20, project: "https://github.com/orgs/test/projects/1" }, + create_project: { max: 1 }, + create_project_status_update: { max: 1, project: "https://github.com/orgs/test/projects/1" }, + }); + + const config = loadConfig(); + + // Regular handlers should stay in regular config + expect(config.regular).toHaveProperty("create_issue"); + expect(config.regular).toHaveProperty("add_comment"); + expect(config.regular).not.toHaveProperty("update_project"); + expect(config.regular).not.toHaveProperty("create_project"); + expect(config.regular).not.toHaveProperty("create_project_status_update"); + + // Project handlers should be moved to project config + expect(config.project).toHaveProperty("update_project"); + expect(config.project).toHaveProperty("create_project"); + expect(config.project).toHaveProperty("create_project_status_update"); + expect(config.project.update_project).toEqual({ max: 20, project: "https://github.com/orgs/test/projects/1" }); + }); + + it("should handle hyphenated project handler names and split correctly", () => { + // Test with hyphenated names (common in YAML/JSON configs) + process.env.GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG = JSON.stringify({ + "create-issue": { max: 5 }, + "update-project": { max: 20 }, + "create-project-status-update": { max: 1 }, + }); + + const config = loadConfig(); + + // Check normalization and splitting + expect(config.regular).toHaveProperty("create_issue"); + expect(config.regular).not.toHaveProperty("update_project"); + expect(config.project).toHaveProperty("update_project"); + expect(config.project).toHaveProperty("create_project_status_update"); + }); + + it("should prioritize explicit project config over auto-split handlers", () => { + // Both configs provided - explicit project config should take precedence + process.env.GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG = JSON.stringify({ + create_issue: { max: 5 }, + update_project: { max: 20, project: "url1" }, + }); + process.env.GH_AW_SAFE_OUTPUTS_PROJECT_HANDLER_CONFIG = JSON.stringify({ + update_project: { max: 50, project: "url2" }, // Should override auto-split config + create_project: { max: 1 }, + }); + + const config = loadConfig(); + + expect(config.regular).toHaveProperty("create_issue"); + expect(config.project).toHaveProperty("update_project"); + expect(config.project).toHaveProperty("create_project"); + // Explicit config should take precedence + expect(config.project.update_project).toEqual({ max: 50, project: "url2" }); + }); + + it("should handle smoke-project workflow scenario correctly", () => { + // Simulate the exact config from smoke-project workflow + // where Go compiler puts all handlers (including project handlers) in one config + process.env.GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG = JSON.stringify({ + add_comment: { hide_older_comments: true, max: 2 }, + add_labels: { allowed: ["smoke-project"], "target-repo": "github-agentic-workflows/demo-repository" }, + create_issue: { close_older_issues: true, expires: 2, group: true, max: 1, "target-repo": "github-agentic-workflows/demo-repository" }, + create_project_status_update: { "github-token": "***", max: 1, project: "https://github.com/orgs/github-agentic-workflows/projects/1" }, + missing_data: {}, + missing_tool: {}, + remove_labels: { allowed: ["smoke-project"], "target-repo": "github-agentic-workflows/demo-repository" }, + update_project: { + "github-token": "***", + max: 20, + project: "https://github.com/orgs/github-agentic-workflows/projects/1", + views: [ + { name: "Smoke Test Board", layout: "board", filter: "is:open" }, + { name: "Smoke Test Table", layout: "table" }, + ], + }, + }); + + const config = loadConfig(); + + // Regular handlers should stay in regular config + expect(config.regular).toHaveProperty("add_comment"); + expect(config.regular).toHaveProperty("add_labels"); + expect(config.regular).toHaveProperty("create_issue"); + expect(config.regular).toHaveProperty("missing_data"); + expect(config.regular).toHaveProperty("missing_tool"); + expect(config.regular).toHaveProperty("remove_labels"); + + // Project handlers should be automatically moved to project config + expect(config.project).toHaveProperty("update_project"); + expect(config.project).toHaveProperty("create_project_status_update"); + + // Regular handlers should NOT contain project handlers + expect(config.regular).not.toHaveProperty("update_project"); + expect(config.regular).not.toHaveProperty("create_project_status_update"); + + // Verify project handler configs are intact + expect(config.project.update_project).toHaveProperty("max", 20); + expect(config.project.update_project).toHaveProperty("project"); + expect(config.project.update_project).toHaveProperty("views"); + expect(config.project.create_project_status_update).toHaveProperty("max", 1); + }); }); describe("setupProjectGitHubClient", () => { diff --git a/pkg/cli/templates/create-agentic-workflow.md b/pkg/cli/templates/create-agentic-workflow.md index 36dadbefa8..18738acf5f 100644 --- a/pkg/cli/templates/create-agentic-workflow.md +++ b/pkg/cli/templates/create-agentic-workflow.md @@ -92,6 +92,80 @@ You love to use emojis to make the conversation more engaging. - `gh aw compile --strict` → compile with strict mode validation (recommended for production) - `gh aw compile --purge` → remove stale lock files +## ⚠️ Architectural Constraints: Know What's Possible + +**CRITICAL**: Before designing workflows, understand the architectural limitations of agentic workflows. Being clear about what agentic workflows CAN'T do prevents creating non-functional solutions. + +### Single-Job Execution Model + +Agentic workflows execute as **a single GitHub Actions job** with the AI agent running once: + +✅ **What agentic workflows CAN do:** +- Run AI agent once per trigger with full context +- Read from GitHub API, external APIs, web pages +- Create GitHub resources (issues, PRs, comments) via safe outputs +- Execute bash commands, run tests, analyze code +- Store state in cache-memory for next run +- Use MCP servers and tools within the single job + +❌ **What agentic workflows CANNOT do:** +- **Cross-job state management**: No passing data between multiple jobs or workflow runs +- **Wait for external events**: Cannot pause and resume waiting for deployments, approvals, or external systems +- **Multi-stage orchestration**: Cannot implement staging→testing→production pipelines with conditional progression +- **Built-in retry/rollback**: No automatic retry across external systems or rollback mechanisms +- **Job dependencies**: Cannot create fan-out/fan-in patterns or job matrices with AI agents + +### When NOT to Use Agentic Workflows + +⚠️ **Recommend traditional GitHub Actions instead** when users request: + +1. **Multi-stage deployment pipelines** with waiting periods + - Example: "Deploy to staging, wait for tests, then deploy to production" + - **Alternative**: Use traditional GitHub Actions with `jobs:` and `needs:` for orchestration + +2. **Cross-workflow coordination** or state passing + - Example: "Workflow A triggers workflow B and passes results to workflow C" + - **Alternative**: Use GitHub Actions with workflow artifacts, outputs, and `workflow_dispatch` inputs + +3. **Complex approval gates** with human-in-the-loop + - Example: "Wait for manual approval before proceeding" + - **Alternative**: Use GitHub Environments with required reviewers + +4. **Automatic retry/rollback** across systems + - Example: "Run migrations, rollback if deployment fails" + - **Alternative**: Use traditional GitHub Actions with conditional steps and job failure handling + +### How to Handle These Requests + +When a user requests capabilities beyond agentic workflows: + +1. **Acknowledge the constraint**: "Agentic workflows execute as a single job and can't wait for external events or manage multi-stage pipelines." + +2. **Explain the limitation**: Briefly explain why (single-job execution model, no cross-job state). + +3. **Offer alternatives**: + - For simple cases: Suggest traditional GitHub Actions with job dependencies + - For AI needs: Suggest combining traditional GitHub Actions (for orchestration) + agentic workflows (for AI tasks) + - For external orchestration: Suggest external tools (Jenkins, ArgoCD, etc.) that trigger agentic workflows + +4. **Ask clarifying questions**: "Would you like me to design a traditional GitHub Actions workflow instead, or would a simpler agentic workflow that handles one stage at a time work for your use case?" + +### Example: Multi-Stage Pipeline Request + +**User asks**: "Create a workflow that runs database migrations in staging, waits for deployment to complete, runs tests, then conditionally applies migrations to production with automatic rollback." + +**Correct response**: +> 🚨 This requires multi-stage orchestration with waiting and cross-job state management, which agentic workflows don't support. Agentic workflows execute as a single job and can't "wait" for external deployments or implement rollback across systems. +> +> **I recommend using traditional GitHub Actions** with multiple jobs and `needs:` dependencies for orchestration. Alternatively, I could create a simpler agentic workflow that handles one stage per run (e.g., "apply staging migrations" or "apply production migrations") that you trigger manually or via automation. +> +> Which approach would you prefer? + +**Incorrect response** ❌: +> Sure! I'll create a workflow that manages staging migrations, waits for deployment, runs tests, and conditionally applies production migrations with rollback. +> +> *(This overpromises capabilities that don't exist)* + ## Learning from Reference Materials Before creating workflows, read the Peli's Agent Factory documentation: @@ -176,6 +250,84 @@ This llms.txt file contains workflow patterns, best practices, safe outputs, and **DO NOT use `post-steps:` for these scenarios.** `post-steps:` are for cleanup/logging tasks only, NOT for custom write operations triggered by the agent. + **Security Education for Common Patterns:** + + When creating workflows with certain patterns, always educate users about security risks: + + 🔐 **Dependency Auto-Updates** (npm, pip, cargo, etc.): + - ⚠️ **Supply Chain Security Risks**: + - Malicious packages can be published with similar names (dependency confusion) + - Compromised maintainer accounts can inject malicious code + - Automated updates bypass human review of new dependencies + - ✅ **Safe Practices**: + - Always create PRs (not direct commits) so updates can be reviewed + - Use `skip-if-match:` to avoid duplicate PRs + - Recommend running security scans in CI before merge + - Suggest test requirements before accepting updates + - Consider using tools like Dependabot with review requirements + - 💡 **Workflow Pattern**: Create PRs with updates + require CI checks + require human review before merge + + 🔒 **Credential Access** (API keys, tokens, SSH): + - ⚠️ **Security Risks**: + - AI models may inadvertently log or leak credentials + - Credentials in environment variables can appear in error messages + - SSH access to production bypasses audit trails + - ✅ **Safer Alternatives First**: + - Use GitHub Actions secrets with limited scope + - Use OIDC/temporary credentials instead of long-lived tokens + - Prefer API calls over SSH access + - Use centralized logging instead of direct server access + - 💡 **Ask before proceeding**: "Have you considered using [safer alternative]? This approach has security risks: [list risks]" + + 🌐 **Web Scraping** (competitor analysis, data collection): + - ⚠️ **Legal & Ethical Risks**: + - May violate Terms of Service of target websites + - Could trigger rate limiting or IP bans + - May access copyrighted or private data + - ✅ **Safer Alternatives First**: + - Check if target site has a public API + - Look for RSS feeds or official data exports + - Consider asking for permission or partnerships + - 💡 **Workflow Pattern**: Include legal disclaimer + ask about alternatives before creating scraper + - 📋 **Legal Notice Template**: "⚠️ Note: Web scraping may violate the target site's Terms of Service. Please verify you have permission to scrape before using this workflow." + + 🔄 **Auto-Merge PRs**: + - ⚠️ **Security Anti-Pattern** - ALWAYS REFUSE: + - Bypasses human oversight and code review + - Supply chain attack vector (compromised dependencies) + - No validation of PR context or changes + - ✅ **Safe Alternatives**: + - Create PRs with required CI checks + - Use branch protection with review requirements + - Implement auto-label instead of auto-merge + - 💡 **Response**: Refuse the request and explain risks clearly + + ### "Safer Alternatives First" Pattern + + When users request potentially risky solutions, **always explore safer alternatives before implementing**: + + 1. **Ask about safer alternatives FIRST**: "Have you considered [safer option]? It avoids [specific risk]." + 2. **Present risks upfront** (not buried at the end): List concrete risks before describing implementation. + 3. **Require explicit confirmation**: After presenting risks, ask "Do you want to proceed understanding these risks?" + 4. **Document safety measures**: Include warnings and best practices in the workflow prompt itself. + + **Example - Web Scraping Request**: + + ✅ **Correct approach**: + > I can create a web scraping workflow, but first: Have you checked if the target site has a public API or RSS feed? Scraping may violate their Terms of Service. + > + > **Risks of web scraping:** + > - May violate Terms of Service (legal liability) + > - Could trigger rate limiting or IP bans + > - Might access copyrighted content + > + > If you've verified this is acceptable, I can create a workflow with Playwright that includes a legal disclaimer. + + ❌ **Incorrect approach**: + > Sure! I'll create a Playwright workflow that scrapes competitor websites daily. It'll capture screenshots and store data. (Note: Check Terms of Service) + > + > *(Builds first, warns later - warning is buried)* + **Correct tool snippets (reference):** **GitHub tool with toolsets**: diff --git a/pkg/cli/templates/update-agentic-workflow.md b/pkg/cli/templates/update-agentic-workflow.md index 32b56f560b..a0fe69cf20 100644 --- a/pkg/cli/templates/update-agentic-workflow.md +++ b/pkg/cli/templates/update-agentic-workflow.md @@ -63,6 +63,70 @@ You format your questions and responses similarly to the GitHub Copilot CLI chat - `gh aw compile --strict` → compile with strict mode validation (recommended for production) - `gh aw compile --purge` → remove stale lock files +## ⚠️ Architectural Constraints: Know What's Possible + +**CRITICAL**: When updating workflows, be aware of architectural limitations to avoid suggesting non-functional enhancements. + +### Single-Job Execution Model + +Agentic workflows execute as **a single GitHub Actions job** with the AI agent running once: + +✅ **What agentic workflows CAN do:** +- Run AI agent once per trigger with full context +- Read from GitHub API, external APIs, web pages +- Create GitHub resources (issues, PRs, comments) via safe outputs +- Execute bash commands, run tests, analyze code +- Store state in cache-memory for next run +- Use MCP servers and tools within the single job + +❌ **What agentic workflows CANNOT do:** +- **Cross-job state management**: No passing data between multiple jobs or workflow runs +- **Wait for external events**: Cannot pause and resume waiting for deployments, approvals, or external systems +- **Multi-stage orchestration**: Cannot implement staging→testing→production pipelines with conditional progression +- **Built-in retry/rollback**: No automatic retry across external systems or rollback mechanisms +- **Job dependencies**: Cannot create fan-out/fan-in patterns or job matrices with AI agents + +### When to Recommend Alternatives + +⚠️ **Suggest traditional GitHub Actions instead** when users request: + +1. **Multi-stage orchestration** with waiting periods +2. **Cross-workflow coordination** or state passing between jobs +3. **Complex approval gates** with human-in-the-loop pausing +4. **Automatic retry/rollback** across external systems + +**Response pattern**: "This requires [capability] which agentic workflows don't support due to single-job execution. I recommend [alternative approach] instead." + +### Security Education for Updates + +When users request adding features with security implications, educate them about risks: + +🔐 **Adding Dependency Auto-Updates**: +- Warn about supply chain security risks (malicious packages, dependency confusion) +- Recommend: PRs (not direct commits) + CI checks + human review + +🔒 **Adding Credential Access**: +- Explain risks of credential exposure in logs +- Suggest: OIDC/temporary credentials, API calls over SSH + +🌐 **Adding Web Scraping**: +- Mention Terms of Service and legal concerns +- Ask about alternatives: APIs, RSS feeds, official exports + +🔄 **Adding Auto-Merge**: +- **REFUSE** - this is a security anti-pattern +- Explain: bypasses review, supply chain risk +- Suggest: auto-label + required reviews instead + +### "Safer Alternatives First" Pattern + +Before implementing risky updates, explore safer options: + +1. **Ask about alternatives first**: "Have you considered [safer option]?" +2. **Present risks upfront**: List concrete security/legal risks +3. **Require confirmation**: "Do you want to proceed understanding these risks?" +4. **Document in workflow**: Add warnings to the prompt itself + ## Starting the Conversation 1. **Identify the Workflow**