fix: use SDK approval kind for tool permissions#268
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR updates the “allow all tools” permission handler to use the Copilot SDK’s approved implementation and adds a regression test to ensure the returned permission result kind matches the SDK constant.
Changes:
- Replace the local
allowAllToolsimplementation with the SDK’sApproveAllhandler. - Add a unit test asserting
allowAllToolsreturnsPermissionRequestResultKindApproved.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/execution/copilot.go | Switches allowAllTools to use the SDK-provided approval handler. |
| internal/execution/copilot_test.go | Adds a test validating the approval kind returned by allowAllTools. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Head branch was pushed to by a user without write access
spboyer
left a comment
There was a problem hiding this comment.
Clean — no issues found. LGTM.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #268 +/- ##
=======================================
Coverage ? 75.78%
=======================================
Files ? 154
Lines ? 17907
Branches ? 0
=======================================
Hits ? 13570
Misses ? 3379
Partials ? 958
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
spboyer
left a comment
There was a problem hiding this comment.
Clean — no issues found. LGTM.
Closes #266
Summary
What Changed
Testing