fix(goose): declare args parameter in generated recipes#2402
fix(goose): declare args parameter in generated recipes#2402natelastname wants to merge 1 commit intogithub:mainfrom
Conversation
mnriem
left a comment
There was a problem hiding this comment.
Can you add some positive and negative tests?
Sure, please let me know if there's anything else I can do. |
There was a problem hiding this comment.
Pull request overview
Fixes Goose YAML recipe generation so recipes that include {{args}} also declare an args parameter, preventing goose recipe validate failures.
Changes:
- Override
GooseIntegration._render_yamlto emit aparameterssection declaringargs. - Add integration tests asserting generated Goose recipes declare
argswhen{{args}}appears in the prompt. - Add a control test confirming the base
YamlIntegrationrenderer does not declareparameters.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/integrations/goose/__init__.py |
Adds Goose-specific YAML rendering that includes an args parameter definition. |
tests/integrations/test_integration_goose.py |
Adds tests verifying the new Goose renderer behavior and documenting the base renderer behavior. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 1
| import re | ||
|
|
|
Please address Copilot feedback |
87641b3 to
5cbe958
Compare
I just edited my previous commit to remove the unused import. |
There was a problem hiding this comment.
Pull request overview
Fixes Goose integration YAML recipe generation so recipes that reference {{args}} declare a corresponding args parameter, preventing goose recipe validate failures.
Changes:
- Override
GooseIntegration._render_yamlto emit aparameterssection containing anargsparameter. - Add Goose-specific tests to ensure generated recipes that contain
{{args}}include anargsparameter declaration.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/integrations/goose/__init__.py |
Overrides YAML rendering to include an args parameter in the recipe header. |
tests/integrations/test_integration_goose.py |
Adds regression tests for args parameter declaration behavior. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 3
|
Please address Copilot feedback. If not applicable, please explain why |
5cbe958 to
ef16b21
Compare
Ok, done |
|
I added an additional check to ensure the ACP “Running recipe” notification only fires when the mapped recipe path actually exists, which brings it in line with the current behavior in Longer term, it might be worth introducing a dedicated event type in the |
Description
Fixes an issue in the Goose integration where generated recipes include
{{args}}in the prompt but do not declare a correspondingargsparameter.This causes
goose recipe validateto fail with:The fix overrides
_render_yamlinGooseIntegrationto include aparameterssection declaring theargsparameter, ensuring all generated recipes are valid.Testing
uv run specify --helpuv sync --extra test && uv run pytestgoose recipe validate)AI Disclosure
Consulted ChatGPT to reason through the root cause and propose an implementation approach. Final code and validation were done manually.