Skip to content

fix(extensions): set-priority repairs corrupted boolean priority#3268

Open
jawwad-ali wants to merge 1 commit into
github:mainfrom
jawwad-ali:fix/extension-set-priority-bool
Open

fix(extensions): set-priority repairs corrupted boolean priority#3268
jawwad-ali wants to merge 1 commit into
github:mainfrom
jawwad-ali:fix/extension-set-priority-bool

Conversation

@jawwad-ali

Copy link
Copy Markdown
Contributor

Description

The extension set-priority command has a skip-if-unchanged guard whose comment promises that corrupted values get repaired:

# This ensures corrupted values (e.g., "high") get repaired even when setting to default (10)
if isinstance(raw_priority, int) and raw_priority == priority:
    console.print(f"...already has priority {priority}")
    raise typer.Exit(0)

But in Python isinstance(True, int) is True and True == 1 (False == 0). So a corrupted boolean priority (e.g. True) matches the guard when the user runs set-priority <ext> 1: the command reports "already has priority 1" and exits without rewriting the bool to a real int — the exact opposite of what the comment promises. normalize_priority() already guards this (if isinstance(value, bool): return default); this call site didn't.

Fix

Exclude bools from the skip guard, mirroring normalize_priority:

if (
    isinstance(raw_priority, int)
    and not isinstance(raw_priority, bool)
    and raw_priority == priority
):

Now a corrupted bool falls through and is repaired to a real int.

Testing

  • New test_set_priority_repairs_corrupted_bool: installs an extension, injects priority: True, runs set-priority test-ext 1, asserts it reports priority changed (not already has priority) and the reloaded value is 1 with not isinstance(..., bool). Fails before (reports "already has priority 1", verified by source-stash), passes after.
  • TestExtensionPriorityCLI passes (8 tests); uvx ruff check clean. The existing same-value test (priority 5, a real int) stays green.

AI Disclosure

  • I did use AI assistance (describe below)

Found and fixed with Claude Code (Claude Opus 4.8) under my direction. AI flagged the bool-is-int trap against the comment's promise; I confirmed the short-circuit, proved fail-before via source-stash, and reviewed the diff before submitting.

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>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants