Skip to content

fix(plugin): make the deprecated plugins/ directory optional#3211

Merged
bpamiri merged 2 commits into
developfrom
peter/optional-plugins-dir-scan
Jun 16, 2026
Merged

fix(plugin): make the deprecated plugins/ directory optional#3211
bpamiri merged 2 commits into
developfrom
peter/optional-plugins-dir-scan

Conversation

@bpamiri

@bpamiri bpamiri commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

The legacy plugins/ directory is deprecated (superseded by vendor/<name>/ packages) and apps are expected to remove it. But two code paths still listed the directory unconditionally and threw when it was absent on engines whose directory listing errors on a missing path — Lucee/Adobe return empty for a missing dir, but stricter engines (e.g. RustCFML) throw, failing onApplicationStart.

Surfaced while benchmarking the pristine Wheels app on RustCFML (directoryList: No such file or directory … /plugins).

Changes

  • Scaffold public/Application.cfc jar-scan (the this.javaSettings.LoadPaths loop): now guards the DirectoryList with DirectoryExists, so a missing plugins/ yields an empty list instead of erroring. Mirrored into the demo app (/public/Application.cfc) and the tweet / starter-app examples.
  • Framework plugin loader vendor/wheels/Plugins.cfc $folders() / $files(): short-circuit to an empty query when the plugins directory doesn't exist.

Behavior is unchanged when plugins/ exists (the scan runs exactly as before). When it's absent, no plugins load and no error is raised. The lookup is deprecated and the inline comments mark it for removal in the next major.

Validation

  • New vendor/wheels/tests/specs/pluginsMissingDirSpec.cfc$init against a non-existent pluginPath does not throw, and $folders() / $files() return empty queries.
  • Full core suite green locally: 4540 passed / 0 failed (Lucee 7 + SQLite).
  • End-to-end: with both guards, RustCFML boots the app cleanly without a plugins/ directory (/ping + /posts → 200, no directory error) — the original failure.

Type of Change

  • Bug fix (cross-engine robustness) + deprecation

Changelog

  • changelog.d/plugins-dir-optional.fixed.md

The legacy plugins/ directory is superseded by vendor/<name>/ packages and
apps are expected to remove it. But two code paths still listed it
unconditionally and threw when it was absent on engines whose directory
listing errors on a missing path (e.g. RustCFML; Lucee/Adobe return empty):

- The scaffold's public/Application.cfc jar-scan (this.javaSettings.LoadPaths
  loop) — guarded with DirectoryExists; mirrored into the demo app and the
  tweet/starter-app examples.
- The framework plugin loader Plugins.cfc $folders()/$files() — now
  short-circuit to an empty query when the plugins directory does not exist.

Behavior is unchanged when plugins/ exists (the scan runs as before); when it
is absent, no plugins load and no error is raised. Adds pluginsMissingDirSpec
(init + $folders()/$files() against a non-existent path). The lookup is
deprecated and slated for removal in the next major.

Signed-off-by: Peter Amiri <petera@pai.com>

@wheels-bot wheels-bot Bot 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.

Wheels Bot — Reviewer

TL;DR. This PR makes the deprecated plugins/ directory optional by guarding every DirectoryList/$directory scan with DirectoryExists, so engines that throw on listing a missing path (e.g. RustCFML) no longer fail at onApplicationStart. The production change is correct across all five Application.cfc copies and both Plugins.cfc helpers, and the changelog fragment + commit message are clean. However, the new spec does not actually exercise the fix — it passes incidentally regardless of whether the guards exist — and the PR's "Validation" claims rest on it. Verdict: request changes (test does not cover the change it claims to validate).

Tests

vendor/wheels/tests/specs/pluginsMissingDirSpec.cfc:29,41,50 call $pluginObj(config), but — unlike all four sibling plugin specs (pluginsSpec.cfc:549, pluginsModernSpec.cfc:982, pluginsSemverSpec.cfc:155, pluginsManifestIntegrationSpec.cfc:455) — this spec never defines a $pluginObj helper:

// what the sibling specs define (and this one is missing):
function $pluginObj(required struct config) {
    return g.$createObjectFromRoot(argumentCollection = arguments.config)
}

Without that helper, $pluginObj(config) does not resolve to "instantiate wheels.Plugins with pluginPath = missingPath." Instead it resolves to the framework's parameterless Global.$pluginObj() (vendor/wheels/Global.cfc:3207), which WheelsTest auto-binds into every spec's variables/this scope from application.wo (vendor/wheels/WheelsTest.cfc:19-38 — it binds every public UDF, and $pluginObj is public). That method ignores the config struct entirely and returns the application's cached PluginObj:

public any function $pluginObj() {           // Global.cfc:3207 — no params
    if (IsDefined("application")) { ... return application[local.appKey].PluginObj; }
    return CreateObject("component", "wheels.Plugins");
}

Consequences:

  1. missingPath is never applied. The returned PluginObj points at the real plugins/ directory, which exists in the repo (it holds a .keep file). So the new if (!DirectoryExists(...)) return QueryNew(...) branches in $folders()/$files() (Plugins.cfc:1049, Plugins.cfc:1098) — the actual fix — are never executed by this spec.
  2. All three assertions still pass, but for the wrong reason: $folders() lists type="dir" on the real dir (0 dirs), $files() filters *.zip (0 files), and the name not like '.%' filter excludes .keep — so recordCount is 0 either way. Delete the DirectoryExists guards entirely and this spec would still go green. It is a no-op test that gives false regression confidence.
  3. The PR body's Validation bullet ("$init against a non-existent pluginPath does not throw, and $folders()/$files() return empty queries") is therefore inaccurate — the spec never constructs Plugins with the non-existent path.

Fix: add the same $pluginObj helper the four sibling specs use, so $createObjectFromRoot(..., method="$init", pluginPath=missingPath) actually builds a Plugins instance bound to the missing path ($createObjectFromRoot at Global.cfc:1048 dispatches the $init method with the full config, including pluginPath). With the helper in place the three assertions will genuinely exercise the new short-circuit branches. As a side benefit it also removes the latent cross-engine risk of passing an extra positional arg to a zero-arg UDF (tolerated on the Lucee 7 you tested, but unverified on Adobe).

Correctness

No issues in the shipped code. The ternary in the Application.cfc files returns [] when the dir is absent, and DirectoryList(..., "path", ...) returns an array, so the for (... in this.wheels.pluginFolders) loop is consistent in both branches. The QueryNew("name,directory,type") early-returns match the columns the callers read — $pluginFolders() (Plugins.cfc:48-50) and $pluginFiles() (Plugins.cfc:88-89) both iterate by recordCount and read name/directory, so an empty query yields an empty result struct with no error.

Cross-engine

Production code is clean: DirectoryExists + ternary and QueryNew are universally supported, and the change only adds a guard in front of an existing scan. The one cross-engine note is folded into the Tests finding above (the broken $pluginObj call's extra-positional-arg behavior was only verified on Lucee 7).

Commits / Docs

fix(plugin): make the deprecated plugins/ directory optional — valid type, suggested scope, lowercase subject, header well under 100 chars. Changelog fragment changelog.d/plugins-dir-optional.fixed.md uses the correct <slug>.fixed.md form rather than editing CHANGELOG.md directly. No user-facing guide or CLAUDE.md convention change is needed for an internal robustness fix. All good here.

The spec called $pluginObj(config) without defining the helper the four
sibling plugin specs use, so it resolved to the parameterless
Global.$pluginObj() that WheelsTest auto-binds — which ignores config and
returns the cached PluginObj pointing at the real plugins/ dir. The
missing-path branch (the $folders()/$files() DirectoryExists guards, the
actual fix) was never executed; the assertions passed for the wrong reason.

Add the same component-level $pluginObj helper the siblings use so
$createObjectFromRoot dispatches $init with pluginPath=missingPath,
building a Plugins instance bound to the non-existent path. The three
assertions now genuinely exercise the guard.

Addresses wheels-bot review on #3211.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Peter Amiri <petera@pai.com>

@wheels-bot wheels-bot Bot 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.

Wheels Bot — Reviewer

TL;DR. This PR makes the deprecated plugins/ directory optional by guarding every DirectoryList/$directory scan with DirectoryExists, so engines that throw on listing a missing path (e.g. RustCFML) no longer fail at onApplicationStart. My earlier review (on 487eedd0c) requested changes because the new spec never defined a $pluginObj helper and so never constructed Plugins with the missing path — the guard branches were never exercised. Commit 9b049afbd adds exactly that helper, and the spec now genuinely runs the fix. No blocking findings remain. Verdict: approve (superseding my prior CHANGES_REQUESTED).

Resolved findings (audit trail)

[Tests — was blocking] The spec did not exercise the fix. Resolved by 9b049afbd.

  • vendor/wheels/tests/specs/pluginsMissingDirSpec.cfc:64-66 now defines the component-level helper, matching the four sibling specs verbatim (pluginsSpec.cfc:549):

    function $pluginObj(required struct config) {
        return g.$createObjectFromRoot(argumentCollection = arguments.config)
    }
  • This helper takes precedence over the auto-bound parameterless Global.$pluginObj(): WheelsTest's pseudo-constructor skips any key already present on the spec (vendor/wheels/WheelsTest.cfc:33if (structKeyExists(variables, local.key) || structKeyExists(this, local.key)) continue;), so the spec's own $pluginObj wins.

  • $pluginObj(config) -> $createObjectFromRoot(..., method="$init", pluginPath=missingPath) (Global.cfc:1048, else-branch Invoke(instance, "$init", config)) now builds a real Plugins instance bound to the missing path. $init computes variables.$class.pluginPathFull from that path (Plugins.cfc:26) and then calls $pluginsExtract() -> $pluginFiles() -> $files() (Plugins.cfc:88) and $pluginsProcess() -> $pluginFolders() -> $folders() (Plugins.cfc:48). Both new if (!DirectoryExists(...)) return QueryNew(...) branches (Plugins.cfc:1049, Plugins.cfc:1098) therefore execute. The three assertions now genuinely cover the change.

Tests

One residual coverage nuance (non-blocking, no action required): on the lenient engines CI runs (Lucee/Adobe), $directory(action="list") returns empty for a missing dir, so these assertions would stay green even if the guards were deleted — a strict fail-without-fix regression test isn't achievable without a strict engine in the matrix. The spec still executes the new branch and documents intent, which is the best available here; the real-world failure was reproduced end-to-end on RustCFML per the PR body.

Correctness

No issues. All five Application.cfc copies use DirectoryExists(dir) ? DirectoryList(...) : [] — the false branch returns an array, matching the for (... in this.wheels.pluginFolders) consumer. The QueryNew("name,directory,type") early-returns match the columns $pluginFolders()/$pluginFiles() read (name/directory), so an empty query yields an empty result with no error.

Cross-engine

Clean. DirectoryExists, the ternary, and QueryNew are universally supported; the change only adds a guard in front of an existing scan. The new spec's $createObjectFromRoot(..., method="$init", ...) path mirrors the four sibling specs that already run across the full engine matrix.

Commits

Both commits conform to commitlint: fix(plugin): make the deprecated plugins/ directory optional and test(plugin): exercise the missing-dir guard via the $pluginObj helper — valid types (fix, test), suggested scope, lowercase subjects, headers well under 100 chars.

Docs

Changelog fragment changelog.d/plugins-dir-optional.fixed.md uses the correct <slug>.fixed.md form rather than editing CHANGELOG.md. No user-facing guide or CLAUDE.md convention change is needed for an internal robustness fix.

@bpamiri bpamiri merged commit b1a7d84 into develop Jun 16, 2026
11 of 12 checks passed
@bpamiri bpamiri deleted the peter/optional-plugins-dir-scan branch June 16, 2026 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant