refactor(cli): unify path normalization, add tests + Windows install docs#2863
Conversation
…docs Follow-up to #2835, which fixed the Windows "no Resource provider [c]" crash (#2841). This does not change the runtime fix #2835 shipped: - Move the canonical normalizePath() into Helpers.cfc (its natural home beside capitalize/pluralize/stripSpecialChars) and reduce Module.$normalizePath() to a one-line delegating wrapper. Helpers is a dependency-free leaf utility, so it is safe to construct during init(). The 6 call sites and the $safeDirExists() fallback are unchanged. - Add 7 normalizePath() regression specs to HelpersSpec.cfc. Because Module delegates to Helpers now, these exercise the real bootstrap path rather than a parallel copy. - Document the Windows "there is no Resource provider available with the name [c]" failure in the CLI installation guide. Supersedes #2843. Verified locally: 667/667 cli/lucli specs pass. Signed-off-by: Peter Amiri <peter@alurium.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Wheels Bot — Reviewer A
This PR consolidates the Windows path-normalisation fix from a private inline copy in Module.cfc into Helpers.normalizePath(), adds 7 regression specs that exercise the real bootstrap code path, and documents the Lucee Resource-API crash for users on stale Windows installs. The refactor is minimal, correctness-preserving, and well-scoped. I have one minor correctness nit about a mid() call with a potentially zero-length count argument, and a small docs observation. No blocking issues.
Correctness
mid(rv, 3, len(rv) - 2) with count 0 when rv is exactly "//"
cli/lucli/services/Helpers.cfc, line 131:
var body = len(leading) ? mid(rv, 3, len(rv) - 2) : rv;When rv = "//" (length 2) and leading = "//", this evaluates to mid("//", 3, 0). CLAUDE.md Cross-Engine Invariant #8 specifically flags Left(str, 0) as a Lucee 7 crash; mid with a count of 0 and a start position past the string end may trigger the same underlying Java string-range error. The path "//" alone is not a legal filesystem path that would reach this function in practice, but a small guard would make the defensive intent explicit:
var body = len(leading) ? (len(rv) > 2 ? mid(rv, 3, len(rv) - 2) : "") : rv;The test suite covers "//server/share/path" and "//server//share" but not the degenerate "//" input. Adding that case would confirm the branch is safe across engines.
Docs
Troubleshooting section mentions only Scoop
web/sites/guides/src/content/docs/v4-0-0/command-line-tools/installation.mdx, new Windows section:
Update to the latest version (`scoop update wheels` for Scoop installs)
The parenthetical qualifier is good — it prevents confusion for non-Scoop users. If the installation guide covers other Windows package managers or a manual binary download, it would be worth noting the equivalent update path (or pointing to the relevant install section) so those users are not left guessing. Low-priority nit; the current wording is accurate.
Commits
Single commit refactor(cli): unify path normalization, add tests + Windows install docs — type refactor, scope cli, 73-char header, sentence-case subject. Conforms to commitlint.config.js. ✓
Tests
Seven specs in HelpersSpec.cfc cover backslash conversion, POSIX no-op, empty input, doubled-slash collapse, drive-letter round-trip, UNC preservation, and UNC doubled-slash collapse. Because Module.$normalizePath() delegates to Helpers.normalizePath(), these specs exercise the actual bootstrap path as claimed. The only gap is the "//" edge case noted above. ✓
Wheels Bot -- Reviewer B (round 1)A's review is solid. No sycophancy, no meaningful false positives, no missed issues on re-scan. The one precision issue is that A invokes Cross-Engine Invariant #8 (which documents SycophancyNone detected. A cited specific files, lines, and engine-specific invariants rather than issuing a blanket approval. False positivesNone. A's Missed issuesNone detected. The PR is CLI-only ( Verdict alignmentA's ConvergenceAligned. A's findings are accurate and appropriately weighted. The PR is a clean, well-tested refactor with limited blast radius. No blocking issues remain. Joint recommendation: approve. |
…ate docs Address Reviewer A on #2863: - Guard the mid(rv, 3, len(rv) - 2) call in Helpers.normalizePath() against a count of 0 when rv is exactly "//" — cf. CLAUDE.md cross-engine invariant #8 (Left(str, 0) crashes Lucee 7; mid() with a zero count and out-of-range start may do the same). This PR introduced the mid() into the bootstrap path via delegation, so the guard sits on the path that actually runs. Output is unchanged for every real input. - Add a regression spec for the degenerate "//" root. - Broaden the Windows troubleshooting note so manual JAR installs get an update path too, not just Scoop. Verified locally: 668/668 cli/lucli specs pass. Signed-off-by: Peter Amiri <peter@alurium.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Wheels Bot — Reviewer A
The new commit (881ec428) directly addresses both findings from the prior round: the mid() guard is in place, the degenerate "//" spec was added, and the Windows troubleshooting note now covers both Scoop and manual JAR installs. No new issues found.
Correctness
Helpers.normalizePath() now reads:
var body = len(leading) ? (len(rv) > 2 ? mid(rv, 3, len(rv) - 2) : "") : rv;Traced through all exercised cases:
"//server/share"— leading"//", bodymid(rv,3,12) = "server/share", collapse no-op →"//server/share"✓"//"— leading"//",len(rv) > 2is false, body"", collapse no-op →"//"✓"C:/Users/tim"— leading"", body = rv, collapse no-op →"C:/Users/tim"✓"/a/b//c"— leading"", body = rv,reReplace("/{2,}", "/", "all")→"/a/b/c"✓
Cross-engine invariant #8 is satisfied — the mid() with count 0 cannot be reached. ✓
One minor observation for the record: normalizePath() does more than the original replace() that $normalizePath() carried privately — it also collapses doubled slashes. This is intentional (the CHANGELOG entry calls it out), but the PR description's "no behavior change" phrasing is a slight simplification. In practice the six call sites in Module.cfc only receive real filesystem paths that won't carry doubled slashes, so the delta is a safe no-op for all real inputs.
Tests
668/668 specs pass. The new "handles a bare double-slash root without a mid() range error" case in HelpersSpec.cfc pins the exact edge that was guarded. ✓
Docs
The troubleshooting paragraph now covers both update paths:
scoop update wheelsfor Scoop installs, or re-fetch the latest Wheels Module (see Manual JAR install above)
Docs nit resolved. ✓
Commits
Both commits conform to commitlint.config.js: type refactor, scope cli, headers 73 and 77 chars, sentence-case subjects. Both carry Signed-off-by trailers. ✓
Wheels Bot — Reviewer B (round 1)A's review on the new SHA is clean. No sycophancy, no false positives, no missed issues. I re-traced the same code paths independently and A's logic holds. SycophancyNone detected. A traced specific code paths, ran the guard through four concrete cases, and noted the subtle "no behavior change" overstatement in the PR description rather than accepting the author's framing uncritically. False positivesNone. I traced all four cases A listed:
The Missed issuesNone detected. The PR is CLI-only code running on LuCLI's bundled Lucee — the cross-engine matrix does not apply. Tests exercise the real bootstrap path via delegation. Both commits carry A's observation that doubled-slash collapse is technically new behavior relative to the private Verdict alignmentA's ConvergenceAligned with A. The PR is a clean, well-tested refactor with limited blast radius. Joint recommendation: approve. |
Summary
Follow-up to #2835, which fixed the Windows
there is no Resource provider available with the name [c]crash (#2841) by normalising backslash paths before they reach Lucee's Resource API. This PR adds the parts #2835 didn't carry — without changing the runtime fix:wheels new#2835 added a private$normalizePath()insideModule.cfc. The canonical implementation now lives inHelpers.normalizePath()— its natural home alongside the other pure string utilities (capitalize,pluralize,stripSpecialChars) — andModule.$normalizePath()is a one-line delegating wrapper.Helpersis a dependency-free leaf utility, so it is safe to instantiate duringinit(). All 6 call sites and the$safeDirExists()java.io.Filefallback from fix(cli): normalize Windows drive-letter paths inwheels new#2835 are untouched.normalizePath()specs inHelpersSpec.cfc. BecauseModule.$normalizePath()delegates toHelpers.normalizePath(), these exercise the actual bootstrap path — not a parallel copy.This is CLI-only code (
cli/lucli/**), so it runs on LuCLI's bundled Lucee — the cross-engine invariants don't apply.Why a follow-up instead of #2843
#2843 proposed a competing, more comprehensive fix for #2841 before #2835 landed. Once #2835 merged, #2843 became largely redundant and conflict-heavy, and a naïve conflict resolution would have reverted #2835's
$safeDirExistsdefense layer. Rather than untangle that, its durable value (tests + docs) is salvaged here against a clean base. #2843 is closed as superseded.Verification
667/667cli/luclispecs pass locally (Lucee 7 + SQLite via LuCLI), including the 7 newnormalizePath()cases — 0 failures, 0 errors.Pull Request Checklist
HelpersSpec.cfcgains anormalizePath()describe block: backslash→forward, POSIX no-op, empty input, doubled-slash collapse, drive-letter prefix round-trip, UNC preservation, UNC doubled-slash collapse.[Unreleased] → Changedentry added.667/667CLI specs locally; CI exercises the full matrix + verb smoke.🤖 Generated with Claude Code