Skip to content

fix: add empty-ID guard to delete_model to prevent wiping engine models directory#100

Merged
BeckettFrey merged 2 commits intoreleasefrom
copilot/add-empty-id-guard-delete-model
Apr 14, 2026
Merged

fix: add empty-ID guard to delete_model to prevent wiping engine models directory#100
BeckettFrey merged 2 commits intoreleasefrom
copilot/add-empty-id-guard-delete-model

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 14, 2026

delete_model did not validate empty engine_id/model_id before path construction. With an empty model_id, _get_model_root resolves to the engine's entire models root directory — which exists — so shutil.rmtree silently deletes every model for that engine. delete_dataset already had this guard; delete_model did not.

Changes

  • src/voxkit/storage/models.py — Added early-return guard at the top of delete_model, matching the delete_dataset pattern:

    if not engine_id or not model_id:
        return False, "Engine ID and Model ID cannot be empty."

    Updated docstring Notes to document the validation.

  • tests/storage/test_models.py — Added test_delete_model_empty_ids covering: empty model_id, empty engine_id, and both empty.

Testing

  • Test testable logic
  • Edge cases addressed

Testing Notes: New test covers all three empty-ID permutations (empty model_id only, empty engine_id only, both empty), asserting success is False and "cannot be empty" in msg.

Documentation

  • README updated (if applicable)
  • Code comments added/updated
  • User-facing documentation updated (if applicable)

Documentation Notes: Docstring Notes section updated to reflect the new validation behavior.

Checklist

  • Code follows project conventions
  • No breaking changes (or documented if intentional)
  • All CI checks pass

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • astral.sh
    • Triggering command: /usr/bin/curl curl -LsSf REDACTED (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title [WIP] Add empty ID guard to delete_model function fix: add empty-ID guard to delete_model to prevent wiping engine models directory Apr 14, 2026
Copilot AI requested a review from BeckettFrey April 14, 2026 21:01
@BeckettFrey BeckettFrey marked this pull request as ready for review April 14, 2026 22:20
@BeckettFrey BeckettFrey merged commit 7ad879e into release Apr 14, 2026
3 checks passed
@BeckettFrey BeckettFrey deleted the copilot/add-empty-id-guard-delete-model branch April 14, 2026 22:20
BeckettFrey added a commit that referenced this pull request Apr 14, 2026
* fixes #64: add ai optimized documentation file

* fix: replace localhost help_url defaults with production URL (#92)

* Initial plan

* fix: replace localhost help_url defaults with production URL

Agent-Logs-Url: https://github.com/BrainBehaviorAnalyticsLab/voxkit-desktop/sessions/496f7ed1-fdaa-4df9-b574-5ab55254c136

Co-authored-by: BeckettFrey <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: BeckettFrey <[email protected]>

* fix: dataset panel empty state non-responsive when splitter resized (#94)

* Initial plan

* fix: prevent helper_label and empty_label from resizing with splitter

Agent-Logs-Url: https://github.com/BrainBehaviorAnalyticsLab/voxkit-desktop/sessions/ede8fa76-ccf4-4940-bd76-7f46adde44c5

Co-authored-by: BeckettFrey <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: BeckettFrey <[email protected]>

* setup-logging (#95)

* Refactor configuration and update release workflow process (#96)

* refactor: remove shadowed config

* replace: move to invoke for os agnostic clarity

* remove release workflow in favor of more manual steps

* Configure shredguard for blocking regex patterns (#98)

* fix: don't mark first launch complete on startup script error (#99)

* Initial plan

* fix: don't mark first launch complete on startup script error

Agent-Logs-Url: https://github.com/BrainBehaviorAnalyticsLab/voxkit-desktop/sessions/e51a7557-a81e-4972-a7e6-45133b115413

Co-authored-by: BeckettFrey <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: BeckettFrey <[email protected]>

* fix: readable_from_unique_id handles prefixed IDs from generate_unique_id (#101)

* Initial plan

* fix: handle prefixed IDs in readable_from_unique_id

Agent-Logs-Url: https://github.com/BrainBehaviorAnalyticsLab/voxkit-desktop/sessions/f6936450-f9a2-4df8-b75a-b403d98db565

Co-authored-by: BeckettFrey <[email protected]>

* fix: raise descriptive ValueError when no timestamp found in readable_from_unique_id

Agent-Logs-Url: https://github.com/BrainBehaviorAnalyticsLab/voxkit-desktop/sessions/f6936450-f9a2-4df8-b75a-b403d98db565

Co-authored-by: BeckettFrey <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: BeckettFrey <[email protected]>

* Initial plan (#103)

Co-authored-by: copilot-swe-agent[bot] <[email protected]>

* Revert "Initial plan (#103)" (#106)

This reverts commit b9ab4a0.

* Fix problem with dedicated internal function (#107)

* fix: add empty-ID guard to delete_model to prevent wiping engine models directory (#100)

* Initial plan

* fix: add empty-ID guard to delete_model to prevent wiping engine models directory

Agent-Logs-Url: https://github.com/BrainBehaviorAnalyticsLab/voxkit-desktop/sessions/0cce5a70-83a5-4c00-b1e8-19f85cb895d1

Co-authored-by: BeckettFrey <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: BeckettFrey <[email protected]>

* fix: validate_dataset checks stem-name pairing between audio and label files (#102)

* Initial plan

* fix: validate_dataset checks stem-name pairing between audio and label files

Agent-Logs-Url: https://github.com/BrainBehaviorAnalyticsLab/voxkit-desktop/sessions/73d34692-65d9-48a2-9621-7127982837a2

Co-authored-by: BeckettFrey <[email protected]>

* chore: remove accidentally committed root conftest.py

Agent-Logs-Url: https://github.com/BrainBehaviorAnalyticsLab/voxkit-desktop/sessions/46f1cfe0-1564-47a8-85ea-4c404b6b23f8

Co-authored-by: BeckettFrey <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: BeckettFrey <[email protected]>

* 82/fix view btn width (#105)

* Fix button width inconsistency

* Migrate to table style

---------

Co-authored-by: Copilot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

delete_model missing empty-ID guard can wipe entire engine models directory

2 participants