Repository navigation
v2.4.0.1 — Gemini code-review fixes for install hygiene (post-ship) - #21
Merged
Merged
Conversation
Post-ship patch covering PRs #19 + #20 — Gemini-flagged fragility in the install-hygiene code that v2.4.0.0 introduced. Mirrors the v2.2.0.0 → v2.2.0.1 precedent for follow-up Gemini review fixes. - Multi-digit version classification now safe for 2.0.10.0, 2.10.0.0, 100.0.0 - New test/test-version-classifier.sh pins the case statement (20 inputs) - setup --keep-legacy and idstack-doctor --local match in any arg position - idstack-doctor: explicit -d check, surfaces silent version-parse failures, bash-native [[ == ]] symlink match - Voice consistency: README/landing page use 3-digit minor for user copy, 4-digit canonical form stays in CHANGELOG/VERSION/plugin.json - CLAUDE.md commands list now mentions bin/idstack-doctor Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request bumps the version to 2.4.0.1 and updates the CHANGELOG.md to reflect several bug fixes related to installation hygiene, version classification, and CLI argument parsing. It also adds the idstack-doctor command to the CLAUDE.md documentation. Feedback was provided regarding the glob pattern used for version classification, which currently fails to account for certain multi-digit major versions like 20.x.
|
|
||
| Two rounds of post-ship Gemini review caught fragility in the install-hygiene code that v2.4.0.0 introduced. All fixes target `setup` and `bin/idstack-doctor`; no skill behavior changes. | ||
|
|
||
| - **Multi-digit version classification.** The `case` statement that decides "modern install — leave alone" vs "pre-v2.0.1.0 — flag for cleanup" used literal-dot + single-digit `[1-9]` patterns that broke on multi-digit components like `2.0.10.0`, `2.10.0.0`, and `100.0.0` — a future v2.0.10.0 install at the legacy path would fall through both arms and produce a false "not recognized" warning. Replaced with multi-digit-safe globs (`2.0.[1-9]*|2.[1-9]*|[3-9]*|1[0-9]*`) mirrored across both files. (Gemini code review of PR #19.) |
There was a problem hiding this comment.
The glob pattern 2.0.[1-9]*|2.[1-9]*|[3-9]*|1[0-9]* mentioned here (and used in the underlying fixes) still misses future major versions in the 20-29, 200-299, etc. ranges. While 1[0-9]* correctly handles versions starting with 1 (like 10.x or 100.x), it does not match those starting with 2 that are not 2.x (e.g., 20.0.0.0). Replacing 1[0-9]* with [1-9][0-9]* would robustly cover all multi-digit major versions.
Suggested change
| - **Multi-digit version classification.** The `case` statement that decides "modern install — leave alone" vs "pre-v2.0.1.0 — flag for cleanup" used literal-dot + single-digit `[1-9]` patterns that broke on multi-digit components like `2.0.10.0`, `2.10.0.0`, and `100.0.0` — a future v2.0.10.0 install at the legacy path would fall through both arms and produce a false "not recognized" warning. Replaced with multi-digit-safe globs (`2.0.[1-9]*|2.[1-9]*|[3-9]*|1[0-9]*`) mirrored across both files. (Gemini code review of PR #19.) | |
| - **Multi-digit version classification.** The `case` statement that decides "modern install — leave alone" vs "pre-v2.0.1.0 — flag for cleanup" used literal-dot + single-digit `[1-9]` patterns that broke on multi-digit components like `2.0.10.0`, `2.10.0.0`, and `100.0.0` — a future v2.0.10.0 install at the legacy path would fall through both arms and produce a false "not recognized" warning. Replaced with multi-digit-safe globs (`2.0.[1-9]*|2.[1-9]*|[3-9]*|[1-9][0-9]*`) mirrored across both files. (Gemini code review of PR #19.) |
3 of 4 tasks
savvides
added a commit
that referenced
this pull request
May 5, 2026
…i on PR #21 (#22) Third Gemini-flagged iteration on the same legacy-version classifier (PR #15 → PR #19 → PR #20 → PR #21). The v2.4.0.1 pattern 1[0-9]* correctly handled multi-digit majors starting with 1 (10–19, 100–199, …) but silently missed any other multi-digit major: 20.x, 25.x, 200.x, etc. fell through both arms of the case and ended up classified as `unknown`. Replaced 1[0-9]* with [1-9][0-9]* in setup, bin/idstack-doctor, and test/test-version-classifier.sh. Strictly more general, no false positives against the legacy arm — [1-9][0-9]* requires a second digit and the legacy arm requires a literal `.` or matches bare `2.0.0`. Test fixture extended with 7 cases that previously fell through to "unknown": 20.0.0.0, 21.5.0, 25.99.0, 29.0.0, 30.0.0, 200.0.0, 999.0.0. Total fixture count: 27. Comment header records the third iteration so future maintainers can see the history. VERSION + plugin.json bumped to 2.4.0.2; CHANGELOG entry added. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Post-ship patch documenting the install-hygiene fixes that landed after v2.4.0.0 was tagged. Mirrors the v2.2.0.0 → v2.2.0.1 precedent for follow-up Gemini code-review fixes.
VERSIONand.claude-plugin/plugin.jsonto2.4.0.1bin/idstack-doctorinto the CLAUDE.md commands list (was missing since v2.4.0.0)No skill behavior changes. No code touched in this PR — all the underlying fixes already landed via #19 and #20 against
main.Documentation
2.4.0.0→2.4.0.1.claude-plugin/plugin.json: version aligned withVERSION(manifest-tracking rule established in v2.4.0.0)### Fixed — install-hygiene follow-ups (Gemini code review). Five bullets covering: multi-digit version classification, the newtest/test-version-classifier.sh, argument-position parsing for--keep-legacy/--local,bin/idstack-doctorrobustness, and voice-consistency in user-facing copy. Plus a### Changednote for the plugin manifest bump.bin/idstack-doctorto the commands block (gap caught during the doc audit — the binary shipped in v2.4.0.0, was already in README troubleshooting and TODOS, but missing from CLAUDE.md).Test plan
cat VERSIONreports2.4.0.1jq -r .version .claude-plugin/plugin.jsonreports2.4.0.1head -3 CHANGELOG.mdshows the new v2.4.0.1 heading./test/smoke-test.shpasses./test/test-version-classifier.shpassesbin/idstack-doctorruns clean against a current install🤖 Generated with Claude Code