Skip to content

Commit d20b452

Browse files
jawwad-aliclaude
andcommitted
fix(extensions): set-priority repairs corrupted boolean priority
The set-priority skip guard 'isinstance(raw_priority, int) and raw_priority == priority' treats a stored boolean as a match because isinstance(True, int) is True and True == 1 (False == 0). So a corrupted boolean priority short-circuits to 'already has priority N' and is never rewritten to a real int — contradicting the adjacent comment that promises corrupted values get repaired. Exclude bools explicitly, mirroring normalize_priority's own bool guard. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 983a87f commit d20b452

2 files changed

Lines changed: 44 additions & 1 deletion

File tree

src/specify_cli/extensions/_commands.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1566,7 +1566,14 @@ def extension_set_priority(
15661566
raw_priority = metadata.get("priority")
15671567
# Only skip if the stored value is already a valid int equal to requested priority
15681568
# This ensures corrupted values (e.g., "high") get repaired even when setting to default (10)
1569-
if isinstance(raw_priority, int) and raw_priority == priority:
1569+
# A bool is an int in Python (isinstance(True, int) is True), so exclude it explicitly —
1570+
# mirroring normalize_priority's bool guard — otherwise a corrupted True/False priority
1571+
# equals 1/0 here and is never repaired.
1572+
if (
1573+
isinstance(raw_priority, int)
1574+
and not isinstance(raw_priority, bool)
1575+
and raw_priority == priority
1576+
):
15701577
console.print(f"[yellow]Extension '{_escape_markup(str(display_name))}' already has priority {priority}[/yellow]")
15711578
raise typer.Exit(0)
15721579

tests/test_extensions.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6424,6 +6424,42 @@ def test_set_priority_same_value_no_change(self, extension_dir, project_dir):
64246424
plain = strip_ansi(result.output)
64256425
assert "already has priority 5" in plain
64266426

6427+
def test_set_priority_repairs_corrupted_bool(self, extension_dir, project_dir):
6428+
"""A corrupted boolean priority must be repaired, not skipped.
6429+
6430+
``isinstance(True, int)`` is True and ``True == 1`` in Python, so a
6431+
stored ``True`` priority would short-circuit the ``already has
6432+
priority 1`` skip path and never get rewritten to a real int —
6433+
contradicting the comment that promises corrupted values are
6434+
repaired. The guard must exclude bools (like normalize_priority).
6435+
"""
6436+
from typer.testing import CliRunner
6437+
from unittest.mock import patch
6438+
from specify_cli import app
6439+
6440+
runner = CliRunner()
6441+
6442+
manager = ExtensionManager(project_dir)
6443+
manager.install_from_directory(
6444+
extension_dir, "0.1.0", register_commands=False, priority=5
6445+
)
6446+
# Inject a corrupted boolean priority (True == 1).
6447+
manager.registry.update("test-ext", {"priority": True})
6448+
6449+
with patch.object(Path, "cwd", return_value=project_dir):
6450+
result = runner.invoke(app, ["extension", "set-priority", "test-ext", "1"])
6451+
6452+
assert result.exit_code == 0, result.output
6453+
plain = strip_ansi(result.output)
6454+
# The corrupted bool must be repaired, not reported as already-set.
6455+
assert "already has priority" not in plain
6456+
assert "priority changed" in plain
6457+
6458+
# The stored value is now a real int, not a bool.
6459+
reloaded = ExtensionManager(project_dir).registry.get("test-ext")
6460+
assert reloaded["priority"] == 1
6461+
assert not isinstance(reloaded["priority"], bool)
6462+
64276463
def test_set_priority_invalid_value(self, extension_dir, project_dir):
64286464
"""Test set-priority rejects invalid priority values."""
64296465
from typer.testing import CliRunner

0 commit comments

Comments
 (0)