feat(skills-manager): confirm destructive actions before applying#75
Conversation
|
This is what would be displayed to the user before updating a Skill (similar for deleting and overwriting): 
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
| result = await event_call( | ||
| { | ||
| "type": "confirmation", | ||
| "data": {"title": title, "message": message}, | ||
| } | ||
| ) |
There was a problem hiding this comment.
To ensure robust compatibility and adherence to the project's async I/O pattern, _request_confirmation should use _call_openwebui_compat instead of calling event_call directly. This prevents potential TypeError exceptions if event_call is a synchronous function.
result = await _call_openwebui_compat(
event_call,
{
"type": "confirmation",
"data": {"title": title, "message": message},
}
)
|
Thanks for the contribution. After addressing the review note about wrapping the confirmation event call with In this repository, the recommended helper is If you are using a Copilot-style agent, you can ask it to use or reference
That should make it easier to bring the PR in line with this repo's release requirements before the next push. |
|
@Fu-Jie thanks for the review! Pushed
One CI note: the |
Fu-Jie
left a comment
Fu-Jie
left a comment
There was a problem hiding this comment.
Code review complete. All 13 files verified. i18n completeness check passed (12 locales). Fixed one security regression in _request_confirmation: replaced bool(result) with a strict isinstance(result, bool) guard so error objects returned on channel disconnect can no longer bypass REQUIRE_CONFIRMATION and allow destructive operations to proceed silently.
Adds a REQUIRE_CONFIRMATION valve (default true) that prompts users via __event_call__ before update_skill, delete_skill, and overwrite paths in create_skill/install_skill. Cancellations return structured results and emit localized status messages; bumps version to 0.3.3 and adds the new strings across all supported locales.
- Route _request_confirmation through _call_openwebui_compat for consistency with other OpenWebUI integration calls. - Sync v0.3.3 across plugin/docs READMEs, docs mirror pages, and root plugin-list badges; bump root updated date to 2026-05-19. - Update What's New blocks in plugin and docs READMEs (EN/CN) and add bilingual v0.3.3 release notes. - Document REQUIRE_CONFIRMATION valve in README_CN.md.
…event bypass on channel error When __event_call__ returns a non-bool truthy value (e.g. an error dict on session disconnect), bool(result) was True, which bypassed REQUIRE_CONFIRMATION and continued executing destructive operations (update, delete, overwrite). Replace bool(result) with an isinstance(result, bool) check so that only an explicit True/False from the confirmation dialog is trusted; any other return is treated as unavailable (returns None) and the caller blocks the action.
Fu-Jie
force-pushed
the
feat/confirm-destructive-skill-actions
branch
from
3942545 to
140b865
Compare
3 participants
Hi! I had to add this confirmation dialog on our end to avoid mistakes — opening this PR in case it would be valuable to the tool as well.
Summary
REQUIRE_CONFIRMATIONvalve (defaulttrue) toopenwebui-skills-managerthat prompts the user via__event_call__beforeupdate_skill,delete_skill, and the overwrite paths increate_skill/install_skill.{cancelled: true, ...}result and emit a localized status; when no interactive event channel is available, callers get a clear error pointing them to the new valve.0.3.3.