-
Notifications
You must be signed in to change notification settings - Fork 4
docs/gen3 draft for wrapped commands/tools #41
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.
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
base: master
Are you sure you want to change the base?
Conversation
Comprehensive Review of CYCOD Custom Tools SpecificationThe specification presents a well-designed framework for extending CYCOD with user-defined shell-based tools. Here's a summary of the key strengths and areas for enhancement from multiple reviews: Key Strengths
Key Enhancement Areas
I'll add specific comments to relevant sections. Please see the uber-critique document for full details. |
| - name: step2 | ||
| bash: command {PARAM} | ||
|
|
||
| # Parameters |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parameter Handling Enhancement Opportunities
-
Type Processing and Validation
- Consider adding validation rules for parameters (min/max for numbers, patterns for strings)
- Add parameter transformation capabilities before substitution
-
Enhanced Documentation
- Parameter descriptions could include examples and detailed help
-
Complex Parameter Types
- Consider supporting object/nested parameters for advanced use cases
See the uber-critique document for specific YAML examples of these enhancements.
robch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Starting a detailed review of the Custom Tools Specification
Parameter Handling Enhancement Opportunities (Lines 54-75)
Example enhancement: parameters:
COUNT:
type: number
description: Count of items
validation:
minimum: 1
maximum: 100
transform: "Math.floor" # Transform function to apply
format: "--count={value}" # How parameter appears in command |
Command Execution Enhancement Opportunities (Lines 76-77)
Example enhancement: commands:
default: find {DIRECTORY} -name "{PATTERN}"
platforms:
windows: powershell -Command "Get-ChildItem -Path '{DIRECTORY}' -Filter '{PATTERN}' -Recurse"
unix: find {DIRECTORY} -name "{PATTERN}"
environment:
variables:
HTTP_PROXY: "{PROXY_URL}"
DEBUG: "1"
inherit: true # Inherit parent process environment |
Error Handling Enhancement Opportunities (Lines 138-153)
Example enhancement: steps:
- name: risky-step
bash: command
error-handling:
retry:
attempts: 3
delay: 1000 # milliseconds
fallback: alternative-command {PARAM}
- name: generate-large-output
bash: large-output-command
output:
max-size: 10MB # Limit output capture size
truncation: true # Whether to truncate if exceeded
streaming: true # Stream output rather than buffering |
Tool Composition and Reusability (Suggested New Section)The specification currently lacks explicit support for:
Example enhancement: steps:
- name: use-git-clone
use-tool: git-clone
with:
URL: "{REPO_URL}"
DIRECTORY: "{WORKSPACE}"
metadata:
category: file-management
subcategory: search
tags: [files, search, find]
keywords: [locate, search, find, pattern]
version: 1.0.0
min-cycod-version: "1.2.0"
changelog:
- version: 1.0.0
changes: "Initial release" |
Implementation Considerations (Suggested New Section)Consider adding sections on:
Example enhancement: tests:
- name: basic-test
parameters:
PATTERN: "*.txt"
DIRECTORY: "test-data"
expected:
exit-code: 0
output-contains: "test-data/sample.txt"
resources:
timeout: 60000 # milliseconds
max-memory: 512MB
cleanup:
- delete-temp-files: true
- final-command: "docker rm {CONTAINER_ID}"
interactive: true # Tool may prompt for user input
interactive-options:
timeout: 30000 # How long to wait for user input
default-response: "y" # Default if no input provided |
Documentation and Usability Enhancements
These improvements would help users create more robust and effective custom tools. |
|
|
||
| Parameter types include: | ||
| - `string`: Text values | ||
| - `number`: Numeric values |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding support for parameter validation rules. For example:
parameters:
COUNT:
type: number
description: Count of items
validation:
minimum: 1
maximum: 100
robch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are some line-specific suggestions to enhance the specification:
Line-Specific Enhancement SuggestionsBased on our review, here are specific line-based suggestions that could be made through the GitHub web interface:
These specific enhancements could be added as line comments in the GitHub web interface. |
Line-Specific Enhancement SuggestionsBased on our review, here are specific line-based suggestions that could be made through the GitHub web interface:
These specific enhancements could be added as line comments in the GitHub web interface. |
Line-Specific Enhancement SuggestionsBased on our review, here are specific line-based suggestions that would be ideal to add directly to specific parts of the code through the GitHub web interface:
See the uber-critique document for detailed YAML examples of these enhancements. |
robch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are some line-specific suggestions for enhancing the Custom Tools Specification
| - `number`: Numeric values | ||
| - `boolean`: True/false values | ||
| - `array`: List of values | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parameter Types Enhancement: Consider adding validation rules for each parameter type. For example:
parameter-types:
string:
validation: [pattern, min-length, max-length]
number:
validation: [minimum, maximum, multiple-of]
boolean:
validation: [allowed-values]
array:
validation: [min-items, max-items, unique-items]|
|
||
| ```yaml | ||
| ignore-errors: false # Whether to continue even if the tool fails | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error Handling Enhancement: Consider adding more sophisticated recovery options like retries and fallbacks:
error-handling:
retry:
attempts: 3
delay: 1000 # milliseconds
fallback: alternative-command {PARAM}
report: true # Whether to include detailed error reports| ```yaml | ||
| platforms: [windows, linux, macos] # Only include platforms the tool works on | ||
| tags: [category1, category2, read] # Tags for categorization and security | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Platform Support Enhancement: Consider supporting platform-specific command variations beyond just compatibility tags:
platforms:
windows:
command: powershell -Command "Get-ChildItem -Path '{DIRECTORY}'"
linux:
command: find {DIRECTORY} -type f
macos:
command: find {DIRECTORY} -type f| - `{step1.output}` - Standard output from the step | ||
| - `{step1.error}` - Standard error from the step | ||
| - `{step1.exit-code}` - Exit code from the step | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Output Management Enhancement: Add specifications for handling large outputs or streaming capabilities:
output-options:
format: json|text|raw # Output format
max-size: 10MB # Maximum capture size
streaming: true # Whether to stream output
transform: jq '.items' # Transform before passing to next step
robch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional line-specific suggestions based on the comprehensive review
| description: Another parameter | ||
| required: false | ||
| default: 5 | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enhanced Parameter Documentation: Consider adding examples and more detailed help for parameters:
parameters:
PATTERN:
type: string
description: File pattern to match
examples: ["*.txt", "log*.log"]
detailed-help: |
A glob pattern that follows standard shell conventions.
Use * as a wildcard for any characters.
Examples: *.txt, log-*.log, data-????.csv| - `number`: Numeric values | ||
| - `boolean`: True/false values | ||
| - `array`: List of values | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Complex Parameter Types: Consider supporting object/nested parameters for advanced use cases:
parameters:
CONFIG:
type: object
properties:
server:
type: string
port:
type: number| - `write`: Tools that modify files or data | ||
| - `run`: Tools that execute other commands or scripts | ||
|
|
||
| If no security tag is provided, the tool is considered high-risk and requires explicit approval. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security Considerations: Consider addressing parameter escaping to prevent command injection:
parameters:
QUERY:
type: string
description: Search query
security:
escape-shell: true # Automatically escape shell metacharacters| run-condition: "{step2.exit-code} == 0" # Condition for when to run this step | ||
|
|
||
| - name: step2 | ||
| bash: command {PARAM} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tool Composition: Consider adding support for tools to reference other tools:
steps:
- name: use-git-clone
use-tool: git-clone
with:
URL: "{REPO_URL}"
DIRECTORY: "{WORKSPACE}"| working-directory: ~/path # Working directory | ||
| platforms: [windows, linux, macos] # Supported platforms | ||
| tags: [tag1, tag2, read] # Categories and security tags | ||
| ignore-errors: false # Whether to continue if a step fails |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tool Discovery and Versioning: Consider adding metadata for better tool discovery and version tracking:
metadata:
category: file-management
subcategory: search
tags: [files, search, find]
keywords: [locate, search, find, pattern]
version: 1.0.0
min-cycod-version: "1.2.0"
changelog:
- version: 1.0.0
changes: "Initial release"| cycod help tools | ||
| ``` | ||
|
|
||
| ## Examples |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tool Testing: Consider adding built-in ways to test tools:
tests:
- name: basic-test
parameters:
PATTERN: "*.txt"
DIRECTORY: "test-data"
expected:
exit-code: 0
output-contains: "test-data/sample.txt"| cycod help tools | ||
| ``` | ||
|
|
||
| ## Examples |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resource Management: Consider adding specifications for resource constraints and cleanup:
resources:
timeout: 60000 # milliseconds
max-memory: 512MB
cleanup:
- delete-temp-files: true
- final-command: "docker rm {CONTAINER_ID}"| cycod help tools | ||
| ``` | ||
|
|
||
| ## Examples |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interactive Mode: Consider adding support for tools that require interactive input:
interactive: true # Tool may prompt for user input
interactive-options:
timeout: 30000 # How long to wait for user input
default-response: "y" # Default if no input provided
robch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Final line-specific suggestion about environment variables
|
|
||
| # Optional settings | ||
| timeout: 60000 # Timeout in milliseconds | ||
| working-directory: ~/path # Working directory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Environment Variables: Consider adding support for setting and managing environment variables for tool execution:
environment:
variables:
HTTP_PROXY: "{PROXY_URL}"
DEBUG: "1"
inherit: true # Inherit parent process environmentThis commit introduces a new documentation file containing examples of custom tools for GitHub PR reviews. The tools include: 1. **PR Summary Comment Tool**: Adds a top-level comment to a PR. 2. **PR Section Comment Tool**: Adds comments referencing specific sections of code. 3. **PR Line Comment Tool**: Adds line-specific comments to a PR. 4. **Advanced PR Multi-Comment Tool**: Allows adding multiple types of comments in a single command. Each tool is accompanied by detailed YAML configurations and usage examples, enhancing the review process with structured commenting capabilities.
No description provided.