From 975396d746cebad97e7bc92560c01991bafe9981 Mon Sep 17 00:00:00 2001 From: jawwad-ali Date: Tue, 30 Jun 2026 21:34:33 +0500 Subject: [PATCH] fix(presets): set-priority repairs corrupted boolean priority MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same bool-is-int trap as the extension set-priority command: the skip guard 'isinstance(raw_priority, int) and raw_priority == priority' treats a stored boolean as a match (isinstance(True, int) is True, True == 1), so a corrupted boolean priority reports 'already has priority N' and is never rewritten to a real int — contradicting the adjacent comment. Exclude bools explicitly, mirroring normalize_priority's bool guard. Co-Authored-By: Claude Opus 4.8 --- src/specify_cli/presets/_commands.py | 9 +++++++- tests/test_presets.py | 34 ++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index eabfe650dd..fbaf479a60 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -461,7 +461,14 @@ def preset_set_priority( raw_priority = metadata.get("priority") # Only skip if the stored value is already a valid int equal to requested priority # This ensures corrupted values (e.g., "high") get repaired even when setting to default (10) - if isinstance(raw_priority, int) and raw_priority == priority: + # A bool is an int in Python (isinstance(True, int) is True), so exclude it explicitly — + # mirroring normalize_priority's bool guard — otherwise a corrupted True/False priority + # equals 1/0 here and is never repaired. + if ( + isinstance(raw_priority, int) + and not isinstance(raw_priority, bool) + and raw_priority == priority + ): console.print(f"[yellow]Preset '{preset_id}' already has priority {priority}[/yellow]") raise typer.Exit(0) diff --git a/tests/test_presets.py b/tests/test_presets.py index 054018b7a0..6db79a6d92 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -4060,6 +4060,40 @@ def test_set_priority_same_value_no_change(self, project_dir, pack_dir): plain = strip_ansi(result.output) assert "already has priority 5" in plain + def test_set_priority_repairs_corrupted_bool(self, project_dir, pack_dir): + """A corrupted boolean priority must be repaired, not skipped. + + ``isinstance(True, int)`` is True and ``True == 1`` in Python, so a + stored ``True`` priority would short-circuit the ``already has + priority 1`` skip path and never get rewritten to a real int — + contradicting the comment that promises corrupted values are + repaired. The guard must exclude bools (like normalize_priority). + """ + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + runner = CliRunner() + + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.5", priority=5) + # Inject a corrupted boolean priority (True == 1). + manager.registry.update("test-pack", {"priority": True}) + + with patch.object(Path, "cwd", return_value=project_dir): + result = runner.invoke(app, ["preset", "set-priority", "test-pack", "1"]) + + assert result.exit_code == 0, result.output + plain = strip_ansi(result.output) + # The corrupted bool must be repaired, not reported as already-set. + assert "already has priority" not in plain + assert "priority changed" in plain + + # The stored value is now a real int, not a bool. + reloaded = PresetManager(project_dir).registry.get("test-pack") + assert reloaded["priority"] == 1 + assert not isinstance(reloaded["priority"], bool) + def test_set_priority_invalid_value(self, project_dir, pack_dir): """Test set-priority rejects invalid priority values.""" from typer.testing import CliRunner