Skip to content

Commit f6f3540

Browse files
jawwad-aliclaude
andauthored
fix(presets): set-priority repairs corrupted boolean priority (#3269)
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 <noreply@anthropic.com>
1 parent 9d96c62 commit f6f3540

2 files changed

Lines changed: 42 additions & 1 deletion

File tree

src/specify_cli/presets/_commands.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,14 @@ def preset_set_priority(
469469
raw_priority = metadata.get("priority")
470470
# Only skip if the stored value is already a valid int equal to requested priority
471471
# This ensures corrupted values (e.g., "high") get repaired even when setting to default (10)
472-
if isinstance(raw_priority, int) and raw_priority == priority:
472+
# A bool is an int in Python (isinstance(True, int) is True), so exclude it explicitly —
473+
# mirroring normalize_priority's bool guard — otherwise a corrupted True/False priority
474+
# equals 1/0 here and is never repaired.
475+
if (
476+
isinstance(raw_priority, int)
477+
and not isinstance(raw_priority, bool)
478+
and raw_priority == priority
479+
):
473480
console.print(f"[yellow]Preset '{preset_id}' already has priority {priority}[/yellow]")
474481
raise typer.Exit(0)
475482

tests/test_presets.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4060,6 +4060,40 @@ def test_set_priority_same_value_no_change(self, project_dir, pack_dir):
40604060
plain = strip_ansi(result.output)
40614061
assert "already has priority 5" in plain
40624062

4063+
def test_set_priority_repairs_corrupted_bool(self, project_dir, pack_dir):
4064+
"""A corrupted boolean priority must be repaired, not skipped.
4065+
4066+
``isinstance(True, int)`` is True and ``True == 1`` in Python, so a
4067+
stored ``True`` priority would short-circuit the ``already has
4068+
priority 1`` skip path and never get rewritten to a real int —
4069+
contradicting the comment that promises corrupted values are
4070+
repaired. The guard must exclude bools (like normalize_priority).
4071+
"""
4072+
from typer.testing import CliRunner
4073+
from unittest.mock import patch
4074+
from specify_cli import app
4075+
4076+
runner = CliRunner()
4077+
4078+
manager = PresetManager(project_dir)
4079+
manager.install_from_directory(pack_dir, "0.1.5", priority=5)
4080+
# Inject a corrupted boolean priority (True == 1).
4081+
manager.registry.update("test-pack", {"priority": True})
4082+
4083+
with patch.object(Path, "cwd", return_value=project_dir):
4084+
result = runner.invoke(app, ["preset", "set-priority", "test-pack", "1"])
4085+
4086+
assert result.exit_code == 0, result.output
4087+
plain = strip_ansi(result.output)
4088+
# The corrupted bool must be repaired, not reported as already-set.
4089+
assert "already has priority" not in plain
4090+
assert "priority changed" in plain
4091+
4092+
# The stored value is now a real int, not a bool.
4093+
reloaded = PresetManager(project_dir).registry.get("test-pack")
4094+
assert reloaded["priority"] == 1
4095+
assert not isinstance(reloaded["priority"], bool)
4096+
40634097
def test_set_priority_invalid_value(self, project_dir, pack_dir):
40644098
"""Test set-priority rejects invalid priority values."""
40654099
from typer.testing import CliRunner

0 commit comments

Comments
 (0)