Skip to content

Add tools.json update step to safe output type custom agent instructions #4297

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 3 commits into from
Nov 18, 2025
Merged

Add tools.json update step to safe output type custom agent instructions #4297

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ff7557e
Initial plan
Copilot Nov 18, 2025
115bdc0
Add Step 3 to update safe_outputs_tools.json in custom agent instruct…
Copilot Nov 18, 2025
7d6a1aa
Add tools.json to Key Success Criteria and Common Pitfalls sections
Copilot Nov 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 84 additions & 16 deletions .github/agents/copilot-add-safe-output-type.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)",
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description field in the example JSON contains usage instructions that should not be part of the actual description field. The parenthetical "(use underscores in name, not hyphens)" should be removed from the description value and kept only in the guidelines below.

The description should be something like:

"description": "Brief description of what this tool does"

The naming convention guidance is already covered in the Tool Signature Guidelines section below.

Suggested change
"description": "Brief description of what this tool does (use underscores in name, not hyphens)",
"description": "Brief description of what this tool does",

Copilot uses AI. Check for mistakes.
"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
Comment on lines +189 to +190
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two guidelines are redundant and say essentially the same thing. Consider consolidating them into a single, clearer guideline:

- Tool `name` must use underscores (e.g., `your_new_type`) and match the safe output type name (converting hyphens to underscores if needed)
Suggested change
- 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
- Tool `name` must use underscores (e.g., `your_new_type`) and match the safe output type name (converting hyphens to underscores if needed)

Copilot uses AI. Check for mistakes.
- 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`

Expand Down Expand Up @@ -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`

Expand Down Expand Up @@ -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`

Expand All @@ -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`

Expand All @@ -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:

Expand Down Expand Up @@ -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`

Expand Down Expand Up @@ -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`
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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

Expand Down
36 changes: 9 additions & 27 deletions pkg/workflow/js/check_skip_if_match.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});

Expand All @@ -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();
});
});
Expand All @@ -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();
});
Expand All @@ -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();
});
Expand All @@ -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");
});
});
Expand All @@ -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();
});
});
Expand Down
Loading