-
Notifications
You must be signed in to change notification settings - Fork 133
fix: Output schema (fix copy-paste mistakes) #2034
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
Conversation
…ainerRunUrl (I guess it is a correct variable)
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.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
Comment @cursor review or bugbot run to trigger another review on this PR
| "type": "string", | ||
| "title": "Chat client", | ||
| "template": "{{run.containerUrl}}" | ||
| "template": "{{run.containerRunUrl}}" |
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.
Bug: Template Variable References Incorrect Object
The template variable {{run.containerRunUrl}} references an incorrect object. Documentation shows {{links.containerRunUrl}} for container URLs, and the run object only contains defaultDatasetId and defaultKeyValueStoreId. This could prevent the template from resolving as expected.
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.
ops, now looking at it again, I'm not sure what is the correct property, @gippy please correct me 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.
You can use either run.containerUrl or links.containerRunUrl.
Both are mentioned in the docs and I think it's confusing, we should unify it.
The links.containerRunUrl is mentioned in the available variables here which is what Cursor picked up here I think.
The run.containerUrl is on the other hand mentioned in the example 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.
Mulled it over a little bit. I think best option is to use run.containerUrl here, remove the links.containerRunUrl from the available template variables section and add the run.containerUrl there instead.
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.
Thank you, fixed
|
Preview for this PR was built for commit |
…ks.containerRunUrl` with `run.containerUrl`
|
Preview for this PR was built for commit |
fix: Output schema (fix copy-paste mistakes) and
containerUrl to containerRunUrl (I guess it is the correct variable)Edit: Replace:
lnkis.containerRunUrlwithrun.containerUrlclose: #2033