diff --git a/.github/agents/copilot-add-safe-output-type.md b/.github/agents/copilot-add-safe-output-type.md index 3f972313fe..dfe7afd577 100644 --- a/.github/agents/copilot-add-safe-output-type.md +++ b/.github/agents/copilot-add-safe-output-type.md @@ -151,7 +151,73 @@ Safe output types are structured data formats that AI agents can emit to perform }; ``` -### Step 3: Update Collection JavaScript +### Step 3: Update Safe Outputs Tools JSON + +**File**: `pkg/workflow/js/safe_outputs_tools.json` + +Add a tool signature for your new safe output type to expose it to AI agents through the MCP server. This file defines the tools that AI agents can call. + +Add a new tool definition to the array: + +```json +{ + "name": "your_new_type", + "description": "Brief description of what this tool does (use underscores in name, not hyphens)", + "inputSchema": { + "type": "object", + "required": ["required_field"], + "properties": { + "required_field": { + "type": "string", + "description": "Description of the required field" + }, + "optional_field": { + "type": "string", + "description": "Description of the optional field" + }, + "numeric_field": { + "type": ["number", "string"], + "description": "Numeric field that accepts both number and string types" + } + }, + "additionalProperties": false + } +} +``` + +**Tool Signature Guidelines**: +- Tool `name` must use underscores (e.g., `your_new_type`), matching the type field in the JSONL output +- The `name` field should match the safe output type name with underscores instead of hyphens +- Include a clear `description` explaining what the tool does +- Define an `inputSchema` with all fields the AI agent will provide +- Use `required` array for mandatory fields +- Set `additionalProperties: false` to prevent extra fields +- For numeric fields, use `"type": ["number", "string"]` to allow both types (agents sometimes send strings) +- Use descriptive field names and descriptions to guide AI agents + +**Examples from existing tools**: +```json +{ + "name": "create_issue", + "description": "Create a new GitHub issue", + "inputSchema": { + "type": "object", + "required": ["title", "body"], + "properties": { + "title": { "type": "string", "description": "Issue title" }, + "body": { "type": "string", "description": "Issue body/description" }, + "labels": { + "type": "array", + "items": { "type": "string" }, + "description": "Issue labels" + } + }, + "additionalProperties": false + } +} +``` + +### Step 4: Update Collection JavaScript **File**: `pkg/workflow/js/collect_ndjson_output.ts` @@ -187,7 +253,7 @@ case "your-new-type": - Use `validateIssueOrPRNumber()` for GitHub issue/PR number fields - Continue the loop on validation errors to process remaining items -### Step 4: Create JavaScript Implementation +### Step 5: Create JavaScript Implementation **File**: `pkg/workflow/js/your_new_type.cjs` @@ -289,7 +355,7 @@ await main(); - Use GitHub Actions context variables for repo information - Follow the existing pattern for environment variable handling -### Step 5: Create Test File +### Step 6: Create Test File **File**: `pkg/workflow/js/your_new_type.test.cjs` @@ -303,7 +369,7 @@ Create comprehensive tests following existing patterns in the codebase: - Use vitest framework with proper mocking - Follow existing test patterns in `.test.cjs` files -### Step 6: Update Collection Tests +### Step 7: Update Collection Tests **File**: `pkg/workflow/js/collect_ndjson_output.test.cjs` @@ -313,7 +379,7 @@ Add test cases for your new type following existing patterns: - Test field type validation - Follow existing test structure in the file -### Step 7: Create Test Agentic Workflows +### Step 8: Create Test Agentic Workflows Create test workflows for each supported engine to validate the new safe output type: @@ -349,7 +415,7 @@ Create a your-new-type output with: Output as JSONL format. ``` -### Step 8: Create Go Job Builder +### Step 9: Create Go Job Builder **File**: `pkg/workflow/your_new_type.go` @@ -551,7 +617,7 @@ func NewPermissionsContentsReadYourPermissions() *Permissions { } ``` -### Step 9: Build and Test +### Step 10: Build and Test 1. **Compile TypeScript**: `make js` 2. **Format code**: `make fmt-cjs` @@ -560,7 +626,7 @@ func NewPermissionsContentsReadYourPermissions() *Permissions { 5. **Compile workflows**: `make recompile` 6. **Full validation**: `make agent-finish` -### Step 10: Manual Validation +### Step 11: Manual Validation 1. Create a simple test workflow using your new safe output type 2. Test both staged and non-staged modes @@ -572,6 +638,7 @@ func NewPermissionsContentsReadYourPermissions() *Permissions { - [ ] JSON schema validates your new type correctly - [ ] TypeScript types compile without errors +- [ ] Safe outputs tools JSON includes the new tool signature - [ ] Collection logic validates fields properly - [ ] JavaScript implementation handles all cases - [ ] Tests achieve good coverage @@ -581,14 +648,15 @@ func NewPermissionsContentsReadYourPermissions() *Permissions { ## Common Pitfalls to Avoid -1. **Inconsistent naming**: Ensure type names match exactly across all files (kebab-case in JSON, camelCase in TypeScript) -2. **Missing validation**: Always validate required fields and sanitize string content -3. **Incorrect union types**: Add your new type to all relevant union types -4. **Missing exports**: Export all new interfaces and types -5. **Test coverage gaps**: Test both success and failure scenarios -6. **Schema violations**: Follow JSON Schema draft-07 syntax strictly -7. **GitHub API misuse**: Use proper error handling for API calls -8. **Staged mode**: Always implement preview functionality for staged mode +1. **Inconsistent naming**: Ensure type names match exactly across all files (kebab-case in JSON, camelCase in TypeScript, underscores in tools.json) +2. **Missing tools.json update**: Don't forget to add the tool signature in `safe_outputs_tools.json` - AI agents won't be able to call your new type without it +3. **Missing validation**: Always validate required fields and sanitize string content +4. **Incorrect union types**: Add your new type to all relevant union types +5. **Missing exports**: Export all new interfaces and types +6. **Test coverage gaps**: Test both success and failure scenarios +7. **Schema violations**: Follow JSON Schema draft-07 syntax strictly +8. **GitHub API misuse**: Use proper error handling for API calls +9. **Staged mode**: Always implement preview functionality for staged mode ## Resources and References diff --git a/pkg/workflow/js/check_skip_if_match.test.cjs b/pkg/workflow/js/check_skip_if_match.test.cjs index cc3dfc3602..f397600808 100644 --- a/pkg/workflow/js/check_skip_if_match.test.cjs +++ b/pkg/workflow/js/check_skip_if_match.test.cjs @@ -107,9 +107,7 @@ describe("check_skip_if_match.cjs", () => { await eval(`(async () => { ${checkSkipIfMatchScript} })()`); - expect(mockCore.setFailed).toHaveBeenCalledWith( - expect.stringContaining("GH_AW_SKIP_QUERY not specified") - ); + expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("GH_AW_SKIP_QUERY not specified")); expect(mockCore.setOutput).not.toHaveBeenCalled(); }); @@ -119,9 +117,7 @@ describe("check_skip_if_match.cjs", () => { await eval(`(async () => { ${checkSkipIfMatchScript} })()`); - expect(mockCore.setFailed).toHaveBeenCalledWith( - expect.stringContaining("GH_AW_WORKFLOW_NAME not specified") - ); + expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("GH_AW_WORKFLOW_NAME not specified")); expect(mockCore.setOutput).not.toHaveBeenCalled(); }); }); @@ -144,9 +140,7 @@ describe("check_skip_if_match.cjs", () => { q: "is:issue is:open label:nonexistent repo:testowner/testrepo", per_page: 1, }); - expect(mockCore.info).toHaveBeenCalledWith( - expect.stringContaining("No matches found") - ); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("No matches found")); expect(mockCore.setOutput).toHaveBeenCalledWith("skip_check_ok", "true"); expect(mockCore.setFailed).not.toHaveBeenCalled(); }); @@ -170,12 +164,8 @@ describe("check_skip_if_match.cjs", () => { q: "is:issue is:open label:bug repo:testowner/testrepo", per_page: 1, }); - expect(mockCore.warning).toHaveBeenCalledWith( - expect.stringContaining("Skip condition matched") - ); - expect(mockCore.warning).toHaveBeenCalledWith( - expect.stringContaining("5 items found") - ); + expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Skip condition matched")); + expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("5 items found")); expect(mockCore.setOutput).toHaveBeenCalledWith("skip_check_ok", "false"); expect(mockCore.setFailed).not.toHaveBeenCalled(); }); @@ -193,9 +183,7 @@ describe("check_skip_if_match.cjs", () => { await eval(`(async () => { ${checkSkipIfMatchScript} })()`); - expect(mockCore.warning).toHaveBeenCalledWith( - expect.stringContaining("1 items found") - ); + expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("1 items found")); expect(mockCore.setOutput).toHaveBeenCalledWith("skip_check_ok", "false"); }); }); @@ -206,18 +194,12 @@ describe("check_skip_if_match.cjs", () => { process.env.GH_AW_WORKFLOW_NAME = "test-workflow"; const errorMessage = "API rate limit exceeded"; - mockGithub.rest.search.issuesAndPullRequests.mockRejectedValue( - new Error(errorMessage) - ); + mockGithub.rest.search.issuesAndPullRequests.mockRejectedValue(new Error(errorMessage)); await eval(`(async () => { ${checkSkipIfMatchScript} })()`); - expect(mockCore.setFailed).toHaveBeenCalledWith( - expect.stringContaining("Failed to execute search query") - ); - expect(mockCore.setFailed).toHaveBeenCalledWith( - expect.stringContaining(errorMessage) - ); + expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Failed to execute search query")); + expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining(errorMessage)); expect(mockCore.setOutput).not.toHaveBeenCalled(); }); });