Skip to content

Commit a63659e

Browse files
savvidesclaude
andauthored
fix(install): version-pattern fragility flagged by Gemini on PR #19 (#20)
Gemini Code Assist correctly noted that the case statement introduced in PR #19 still uses literal-dot + single-digit [1-9] patterns, which fail on multi-digit components like 2.0.10.0, 2.10.0.0, 100.0.0. A future v2.0.10.0 install at the legacy path would fall through both arms and produce a false-positive "not a recognized install" warning. Replace the skip arm with multi-digit-safe patterns: 2.0.[1-9]*|2.[1-9]*|[3-9]*|1[0-9]* Refinement vs Gemini's exact suggestion: 1[0-9]* (no literal dot) instead of 1[0-9].* — covers 100.0.0 too, no false positives since the legacy arm matches `1.*` not `1[0-9]`. Mirrored in bin/idstack-doctor and setup. Adds test/test-version-classifier.sh — pins the case statement against 20 representative versions (legacy + modern + multi-digit). Wired into smoke-test.sh. This is the second time Gemini caught a version-pattern bug in this code path; the test stops the third. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent cbf32e9 commit a63659e

4 files changed

Lines changed: 91 additions & 5 deletions

File tree

bin/idstack-doctor

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,11 @@ elif [ -d "$LEGACY_DIR" ]; then
107107
if [ -f "$LEGACY_DIR/VERSION" ]; then
108108
v=$(tr -d '[:space:]' < "$LEGACY_DIR/VERSION")
109109
# Two arms: explicitly skip modern/future versions first, then flag the
110-
# legacy ones. Mirrors setup; future-proof if v3.x or v10.x ever ships.
110+
# legacy ones. Mirrors setup. Patterns avoid literal dots and single-digit
111+
# ranges so multi-digit components (2.0.10.0, 2.10.0.0, 100.0.0) work.
112+
# Covered by test/test-version-classifier.sh.
111113
case "$v" in
112-
2.0.1.*|2.[1-9].*|[3-9].*|1[0-9].*) ;;
114+
2.0.[1-9]*|2.[1-9]*|[3-9]*|1[0-9]*) ;;
113115
0.*|1.*|2.0.0.*|2.0.0) signature="${signature:+$signature, }VERSION=$v" ;;
114116
esac
115117
fi

setup

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,11 @@ elif [ -d "$LEGACY_SKILLS_LINK" ] && [ "$LEGACY_SKILLS_LINK" != "$IDSTACK_DIR" ]
7070
if [ -f "$LEGACY_SKILLS_LINK/VERSION" ]; then
7171
legacy_version=$(tr -d '[:space:]' < "$LEGACY_SKILLS_LINK/VERSION")
7272
# Two arms: explicitly skip modern/future versions first, then flag the
73-
# legacy ones. Self-documenting and future-proof if a v3.x or v10.x ever
74-
# ships through this path.
73+
# legacy ones. Patterns avoid literal dots and single-digit ranges so
74+
# multi-digit components (2.0.10.0, 2.10.0.0, 100.0.0) work.
75+
# Covered by test/test-version-classifier.sh.
7576
case "$legacy_version" in
76-
2.0.1.*|2.[1-9].*|[3-9].*|1[0-9].*) ;;
77+
2.0.[1-9]*|2.[1-9]*|[3-9]*|1[0-9]*) ;;
7778
0.*|1.*|2.0.0.*|2.0.0) legacy_is_old_version=1 ;;
7879
esac
7980
fi

test/smoke-test.sh

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,13 @@ if [ -x "$IDSTACK_DIR/test/test-manifest-merge.sh" ]; then
111111
check "manifest-merge unit tests pass" "'$IDSTACK_DIR/test/test-manifest-merge.sh'"
112112
fi
113113

114+
# Version classifier (shared by setup + bin/idstack-doctor) must classify
115+
# multi-digit versions correctly. Pinned to catch the pattern-fragility
116+
# regression Gemini flagged twice.
117+
if [ -x "$IDSTACK_DIR/test/test-version-classifier.sh" ]; then
118+
check "version-classifier unit tests pass" "'$IDSTACK_DIR/test/test-version-classifier.sh'"
119+
fi
120+
114121
# Check generated files have auto-generated header
115122
for skill in $SKILLS; do
116123
check "$skill SKILL.md has auto-generated header" "grep -q 'AUTO-GENERATED from SKILL.md.tmpl' '$IDSTACK_DIR/skills/$skill/SKILL.md'"

test/test-version-classifier.sh

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
#!/usr/bin/env bash
2+
# Unit tests for the legacy-VERSION classifier shared by setup and
3+
# bin/idstack-doctor. Both files use the same two-arm case statement to
4+
# decide whether a VERSION file under the legacy install path means
5+
# "modern install — leave alone" (skip) or "pre-v2.0.1.0 install — flag
6+
# for cleanup" (legacy).
7+
#
8+
# Why this test exists: Gemini Code Assist has flagged this case statement
9+
# twice now. The patterns are subtle (bash globs, not regex) and an off-by-one
10+
# in a character class silently flips classifications for multi-digit
11+
# components. This test pins the contract so it can't drift again.
12+
#
13+
# Run from the repo root or via smoke-test.sh.
14+
15+
set -e
16+
17+
PASS=0
18+
FAIL=0
19+
TOTAL=0
20+
21+
# Mirror of the case statement in setup and bin/idstack-doctor. Keep these
22+
# patterns in lockstep with both files — if you change one, change all three.
23+
classify_version() {
24+
case "$1" in
25+
2.0.[1-9]*|2.[1-9]*|[3-9]*|1[0-9]*) echo "skip" ;;
26+
0.*|1.*|2.0.0.*|2.0.0) echo "legacy" ;;
27+
*) echo "unknown" ;;
28+
esac
29+
}
30+
31+
check() {
32+
TOTAL=$((TOTAL + 1))
33+
local version="$1"
34+
local expected="$2"
35+
local got
36+
got="$(classify_version "$version")"
37+
if [ "$got" = "$expected" ]; then
38+
PASS=$((PASS + 1))
39+
echo " PASS: $version -> $got"
40+
else
41+
FAIL=$((FAIL + 1))
42+
echo " FAIL: $version -> $got (expected $expected)"
43+
fi
44+
}
45+
46+
echo "test-version-classifier"
47+
echo ""
48+
49+
# Legacy versions that ever shipped.
50+
check "0.1.0" legacy
51+
check "0.5.0" legacy
52+
check "1.0.0" legacy
53+
check "1.9.0" legacy
54+
check "2.0.0" legacy
55+
check "2.0.0.0" legacy
56+
check "2.0.0.1" legacy
57+
58+
# Modern versions — including multi-digit components that broke the
59+
# previous patterns.
60+
check "2.0.1.0" skip
61+
check "2.0.1.5" skip
62+
check "2.0.10.0" skip
63+
check "2.0.99.0" skip
64+
check "2.1.0.0" skip
65+
check "2.4.0.0" skip
66+
check "2.10.0.0" skip
67+
check "2.99.0.0" skip
68+
check "3.0.0.0" skip
69+
check "9.0.0.0" skip
70+
check "10.0.0.0" skip
71+
check "19.0.0.0" skip
72+
check "100.0.0.0" skip
73+
74+
echo ""
75+
echo " $PASS/$TOTAL passed"
76+
[ "$FAIL" = "0" ] || exit 1

0 commit comments

Comments
 (0)