Repository navigation
fix(packages): use java.io.File.mkdirs() for cache dirs so Adobe CF works - #2598
Conversation
…orks ManifestCache.$ensureDir used DirectoryCreate(path, true). The createPath flag is a Lucee extension — Adobe CF rejects the second argument and crashes the Tools → Packages page on every fresh ACF install with "The function takes 1 parameter" (#2567). Routes the recursive mkdir through java.io.File.mkdirs(), which has stable JVM-level semantics on Lucee, Adobe CF, and BoxLang. Mirrors the fix in the CLI-side ManifestCache so the two intentionally-paired files stay in sync. Adds regression coverage exercising a deeply-nested cache root (where parents don't exist yet — the case the createPath flag was meant to cover) and the on-first-use manifests subdirectory creation path. Also documents the BIF gotcha in .ai/wheels/cross-engine-compatibility.md so the next person reaches for Java NIO before tripping on it. Closes #2567
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: PR #2598 fixes a real cross-engine crash — DirectoryCreate(path, true) is a Lucee-only extension that Adobe CF rejects with "The function takes 1 parameter" on fresh installs, breaking the Tools → Packages page. The fix routes through java.io.File.mkdirs(), which is a JDK API and behaves identically on all three engines. The implementation is correct, the tests are well-structured BDD specs that cover both the multi-level-parent regression and the manifests/ subdirectory path, and the cross-engine-compatibility doc is updated appropriately. I have two minor findings — one robustness nit and one missing changelog entry — neither is blocking.
Correctness
mkdirs() return value is silently discarded
vendor/wheels/services/packages/ManifestCache.cfc line 129 (and the mirror at cli/lucli/services/packages/ManifestCache.cfc line 123):
CreateObject("java", "java.io.File").init(arguments.path).mkdirs();java.io.File.mkdirs() returns boolean: true if all directories were created, false if they already existed or if creation failed (e.g. permissions, read-only filesystem). Discarding this silently means a permission failure becomes a confusing FileWrite error a few lines later rather than a clear "could not create cache directory" message.
Suggested improvement (not a hard blocker since the subsequent FileWrite will still throw, but the diagnostic message will be opaque):
var created = CreateObject("java", "java.io.File").init(arguments.path).mkdirs();
if (!created && !DirectoryExists(arguments.path)) {
Throw(
type = "Wheels.Packages.CacheDir",
message = "Could not create cache directory: " & arguments.path
);
}The !DirectoryExists guard handles the case where mkdirs() returns false because the directory already appeared between the initial existence check and the mkdirs() call — that race outcome is harmless.
Docs
Missing CHANGELOG entry
The [Unreleased] → ### Fixed section in CHANGELOG.md (line 155) has entries for recent fixes but nothing for this PR. Other Adobe CF compatibility fixes in the same release have been recorded there. A one-liner is all it needs:
- Tools → Packages page no longer 500s on fresh Adobe CF installs. `$ensureDir` in `ManifestCache`
called `DirectoryCreate(path, true)` whose `createPath` argument is a Lucee extension rejected by
Adobe CF. Replaced with `java.io.File.mkdirs()` which recurses parents identically on every engine.
(#2567)Tests
Both the framework spec (vendor/wheels/tests/specs/packages/ManifestCacheEnsureDirSpec.cfc) and the CLI addition (cli/lucli/tests/specs/packages/ManifestCacheSpec.cfc lines 74-89) correctly use WheelsTest BDD syntax, cover the regression case (multi-level parents), and use try/finally for temp-dir cleanup. The CreateUUID()-based uniqueness avoids test-run collisions. No concerns here.
Cross-engine
The fix is specifically motivated by cross-engine compatibility and is correct. java.io.File.mkdirs() is a core JDK API present on every JVM-hosted CFML engine (Lucee, Adobe CF, BoxLang). The .ai/wheels/cross-engine-compatibility.md entry documents the pattern clearly and will prevent future contributors from re-introducing the same BIF call. No further cross-engine issues detected.
Commits
fix(packages): use java.io.File.mkdirs() for cache dirs so Adobe CF works — valid type, scope, and subject. Under 100 chars. Not ALL-CAPS. Conforms to commitlint.config.js.
Wheels Bot — Reviewer B (round 1)A's review is technically sound. Both findings are accurate, no false positives were detected, and a re-scan of the diff found no missed issues. A's COMMENTED (non-blocking) verdict is appropriate for a PR whose core fix, tests, and documentation are all correct. Converging on approve. SycophancyNone detected. A led with a substantive technical summary, identified two concrete issues with file/line citations, and submitted a COMMENTED state rather than a blanket approval — none of the usual sycophancy signals. False positivesNone detected.
Missed issuesNone detected. The diff is tightly scoped — two identical function rewrites, two BDD spec additions, one doc entry. Test files use WheelsTest BDD syntax correctly, cleanup is in Verdict alignmentA's COMMENTED verdict (non-blocking) is consistent with the findings. Both are quality improvements on an otherwise correct PR — neither describes a behavioral defect or a cross-engine regression. Submitting COMMENTED rather than APPROVED is defensible; submitting APPROVED with optional suggestions would also have been fine. ConvergenceAligned. A's two findings are valid and the PR's core implementation is sound. Neither finding constitutes a blocker. Joint recommendation: approve; the mkdirs error-handling and CHANGELOG entry are worth the author picking up but should not gate merge. |
…log entry Address Reviewer A on #2598: - $ensureDir now captures mkdirs()'s boolean return and throws a typed Wheels.Packages.CacheDir exception when creation actually failed. Without this guard, a permission/read-only-fs failure surfaced as a misleading downstream FileWrite error rather than a clear "could not create cache directory" diagnostic. The !DirectoryExists re-check swallows the benign concurrent-creation race where mkdirs() returns false because another worker won. - CHANGELOG entry under [Unreleased] → Fixed describing the user-facing symptom (Tools → Packages 500 on fresh ACF), root cause, and the cross-engine fix, mirroring the format of the surrounding entries.
…lin-55d865 # Conflicts: # CHANGELOG.md
|
Both findings addressed in 164cdfcdd:
b1941ab55 merges develop and resolves a parallel CHANGELOG insert from #2597. |
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: This is a follow-up iteration on PR #2598. The previous Reviewer A round (commit 3088309a8) flagged two issues: the mkdirs() boolean return was silently discarded, and no CHANGELOG entry existed. Both have been addressed in the follow-up commit 164cdfcdd. The implementation is now correct, well-documented, and cross-engine safe. Approving.
Correctness
Both previous findings are resolved.
mkdirs() return value captured and checked (vendor/wheels/services/packages/ManifestCache.cfc line 132, CLI mirror at cli/lucli/services/packages/ManifestCache.cfc line 126):
local.created = CreateObject("java", "java.io.File").init(arguments.path).mkdirs();
if (!local.created && !DirectoryExists(arguments.path)) {
Throw(
type = "Wheels.Packages.CacheDir",
message = "Could not create cache directory '#arguments.path#'."
);
}The !DirectoryExists re-check correctly handles the benign race where mkdirs() returns false because another thread created the directory concurrently — that outcome is not a failure. A genuine permission/read-only-fs failure now throws Wheels.Packages.CacheDir immediately rather than producing an opaque downstream FileWrite error. The typed exception makes it catchable at call sites if needed.
Cross-engine
No concerns. java.io.File.mkdirs() is a JDK API available on Lucee, Adobe CF, and BoxLang identically. No Lucee-only BIFs remain in the $ensureDir path.
Tests
vendor/wheels/tests/specs/packages/ManifestCacheEnsureDirSpec.cfc extends wheels.WheelsTest and covers two distinct cases:
- Multi-level parent creation (the original
createPath=trueregression case). manifests/subdirectory creation on firstwriteManifestcall.
Both use try/finally for cleanup, CreateUUID()-based uniqueness to avoid test collisions, and correct BDD syntax. The CLI parallel case in ManifestCacheSpec.cfc lines 74-89 mirrors the same scenario with a simpler path-cleanup strategy (saving unique before constructing the nested path, which is marginally cleaner than the framework spec's ListFirst(Replace(...)) approach — worth harmonising in a follow-up, but not a blocker).
Docs
CHANGELOG (CHANGELOG.md line 158): Entry is present, placed correctly under [Unreleased] → Fixed, describes the user-visible symptom (Tools → Packages 500), root cause, and fix. Matches the verbosity and style of surrounding entries.
.ai cross-engine doc (.ai/wheels/cross-engine-compatibility.md lines 128-142): The new entry clearly flags DirectoryCreate(path, true) as a Lucee-only extension, shows the wrong and right patterns, and explains why Java NIO is the correct choice. Future contributors will hit this before tripping on the BIF.
Commits
Both commits conform to commitlint.config.js:
fix(packages): use java.io.File.mkdirs() for cache dirs so Adobe CF works— valid type, scope, subject under 100 chars.fix(packages): throw typed exception when mkdirs() fails + add changelog entry— same. The body explains the "why" clearly (diagnostic clarity on permission failures, race-condition guard reasoning).
Wheels Bot — Reviewer B (round 1)A's review is accurate and its APPROVED verdict is well-earned. Both findings from the previous round were addressed in the follow-up commits; A verified each one against the actual diff lines. No sycophancy, one very minor doc observation that A omitted, but it does not affect verdict alignment. SycophancyNone detected. A opened with a concrete summary of the previous round's state, cited specific file paths and line numbers for both verified findings, and submitted APPROVED only after confirming the fixes rather than on the basis of the PR description alone. False positivesNone detected.
Missed issuesOne minor doc gap, not a blocker:
Verdict alignmentA's APPROVED verdict is consistent with the findings. Both prior actionable items are addressed; the remaining observation (doc example completeness) is non-blocking. Approval is appropriate. ConvergenceAligned. A's review is clean: no unresolved missed issues, no false-positive disputes, and APPROVED is the correct verdict for a follow-up iteration that addressed the consensus findings from the previous round. Joint recommendation: approve and merge. |
Summary
ManifestCache.$ensureDirwas callingDirectoryCreate(path, true). ThecreatePathflag is a Lucee extension — Adobe CF rejects the second argument with"The function takes 1 parameter", crashing the Tools → Packages page on every fresh ACF install (Packages page fails on Adobe ColdFusion:DIRECTORYCREATEcalled with unsupported arguments #2567).java.io.File.mkdirs(), which has stable JVM-level semantics on Lucee, Adobe CF, and BoxLang. Same pattern as the existingCreateObject("java", "java.lang.System")call already in this file.cli/lucli/services/packages/ManifestCache.cfc, the deliberate mirror called out in the framework file's docstring.Why the bot's "just drop the second argument" sketch wasn't enough
The wheels-bot triage on the issue suggested either dropping the second arg or branching on engine, claiming "Adobe CF creates parents by default." That's incorrect — Adobe CF's
DirectoryCreate(path)only creates the leaf. If~/.wheels/doesn't yet exist (fresh install), a single-argument call would still fail on every engine. Java NIO bypasses the BIF entirely and recurses parents the same way everywhere.Regression coverage
vendor/wheels/tests/specs/packages/ManifestCacheEnsureDirSpec.cfc— new spec exercising the multi-level-parent case the originalcreatePath=truewas meant to cover, plus themanifests/subdirectory creation path. Runs on every engine in the compat-matrix.it()block to the existingcli/lucli/tests/specs/packages/ManifestCacheSpec.cfcso the CLI mirror keeps the same coverage.Docs
.ai/wheels/cross-engine-compatibility.mdgains a new entry documenting the BIF gotcha so the next contributor reaches for Java NIO before tripping on it.Test plan
ManifestCacheEnsureDirSpecis green in the core suite for every engine × DB comboManifestCacheSpec.itdeep-nested case is green in the CLI suite on Lucee 7Closes #2567