- Notifications
You must be signed in to change notification settings - Fork24
feat: addprompt andapp_id fields tocoder_ai_task#445
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
0a8c610 tobf7817eComparetask_id,prompt andapp_id fields tocoder_ai_taskprompt andapp_id fields tocoder_ai_taskThere 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.
Pull Request Overview
This PR adds support for task identification and prompt handling in thecoder_ai_task resource by introducing new fields and deprecating the existingsidebar_app configuration pattern.
- Adds required
app_idfield and computedtask_id/promptfields to thecoder_ai_taskresource - Deprecates the
sidebar_appfield in favor of the simplerapp_idapproach - Updates tests to cover the new field validation and backward compatibility scenarios
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| provider/provider.go | Updates resource function name fromaiTask() toaiTaskResource() |
| provider/ai_task.go | Implements new fields with environment variable integration and backward compatibility logic |
| provider/ai_task_test.go | Adds comprehensive test coverage for new fields, validation, and deprecation scenarios |
| docs/resources/ai_task.md | Updates documentation to reflect the new optional/computed fields and deprecation |
Tip: Customize your code reviews with copilot-instructions.md.Create the file orlearn how to get started.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Closescoder/internal#977Adds one required field `app_id`, one read-only computed field `prompt`,as well as deprecates the `sidebar_app` field.Also updates the `id` field to inherit the value from `CODER_TASK_ID`env variable
bf7817e toa8d0534Compare| }else { | ||
| resourceData.SetId(uuid.NewString()) | ||
| } |
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.
Would it make more sense to exit with an error here?
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.
I don't mind either.
What would happen if the provider version this lands in is used with an older version of coder? (I guess also do we care?)
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.
The integration tests will tell us :)
I'd expect that it won't have any effect unless the template defines acoder_ai_task. In that case, folks can lock their provider version.
mafredri 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.
LGTM!
| } | ||
| ifprompt:=os.Getenv("CODER_TASK_PROMPT");prompt!="" { | ||
| resourceData.Set("prompt",prompt) |
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.
Question, prompt vs input? I don't personally mind either way as both work. "You give your task an initial prompt and then send new input".
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.
The RFC laid out prompt so I'm tempted to keep it here, I also don't mind either way.
Uh oh!
There was an error while loading.Please reload this page.
63d905c to347e659Comparec2c443b intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Closescoder/internal#977
Adds one required field
app_id, one read-only computed fieldprompt,as well as deprecates the
sidebar_appfield.Also updates the
idfield to inherit the value fromCODER_TASK_IDenv variable