fix(cli): reload and packages add no longer claim onApplicationStart won't re-fire#3126
Conversation
…won't re-fire An authorized ?reload=true&password=... calls applicationStop(), so the next request re-fires onApplicationStart in full — config/services.cfm and the PackageLoader ($loadPackages) all re-run (verified live on Lucee 7, #3110). The reload command's note and the packages-install activation line claimed the opposite, telling users to run `wheels stop && wheels start`. Correct both: the reload note now describes the real full-restart behavior and surfaces the password-resolution caveat (a missing/wrong password silently skips the restart, #3059 / #3062); the install output now says `wheels reload` (or restart) activates the package. Tutorial/guide passages making the same claim are handled separately by bot-update-docs.yml. Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR: This PR corrects two CLI messages (wheels reload note in cli/lucli/Module.cfc and the packages add activation hint in cli/lucli/services/packages/PackagesMainCli.cfc) that claimed ?reload=true does not re-fire onApplicationStart. I independently verified the corrected contract against the framework source and it is accurate; the specs were checked against the head source and their assertions hold (and would deterministically fail against the old source, supporting the TDD claim). Verdict: comment — no blocking findings, one non-blocking test-quality nit.
Verification performed
- The corrected claim is true.
public/Application.cfc:272-295gates?reload=trueon the configuredreloadPassword(constant-time compare) and routes to$handleRestartAppRequest(), which callsapplicationStop()atpublic/Application.cfc:473. On the next request,vendor/wheels/events/onapplicationstart.cfc:380-385re-includesconfig/services.cfm(plus the environment override) and line 415 calls$loadPackages()— exactly what the new messages say. The password caveat is also correct: when a configured password doesn't match, the gate condition at lines 273-284 is false and the request is served normally with no restart and no error. - Spec assertions verified against head.
Module.cfcno longer containsonApplicationStart does NOT re-fireanywhere (the remainingwheels stop && wheels startatModule.cfc:901is the unrelatedstopcommand hint and is no longer asserted against).$doInstall's return string (PackagesMainCli.cfc:218-219) containswheels reloadand notwheels stop && wheels start; the new code comment splits that phrase across lines 214-215, butPackagesMainCliSpeccorrectly asserts on the return string, not the source, so there is no false-pass risk there. - Commit & changelog. Single commit
fix(cli): reload and packages add no longer claim onApplicationStart won't re-fire— valid type, header under 100 chars, body explains the why, DCO sign-off present. Changelog fragmentchangelog.d/3110-reload-refire-contract.fixed.mdfollows the<slug>.fixed.mdconvention with a complete bullet line.
Tests
- Non-blocking nit —
cli/lucli/tests/specs/commands/ReloadCommandSpec.cfc:24:expect(moduleSource).toInclude("re-fires onApplicationStart")scans the whole file and is currently satisfied by the code comment atModule.cfc:843as well as the user-facingout()call atModule.cfc:849. If a future change deleted theout()line but kept the comment, this spec would still pass while the user-visible note disappeared. Suggest anchoring to the output string itself, e.g.toInclude("Note: an authorized reload re-fires onApplicationStart")— that matches only theout()line. The source-scan approach itself is fine (it is the pre-existing pattern for this spec, acknowledged in its header), and thenotToIncluderegression guard at line 26 is solid either way.
Docs
- Deferring the tutorial/guide passages (
06-authentication.mdx,08-bonus-basecoat.mdx) tobot-update-docs.ymlis reasonable, and rootCLAUDE.mdalready states the correct contract for package activation ("Restart orwheels reloadafter install"), so no in-repo doc drift is introduced by this PR. No finding.
A non-blocking follow-up thought (pre-existing, out of scope here): reload() prints "Application reloaded successfully." whenever the HTTP request succeeds — including when a missing/wrong password caused the restart to be silently skipped (the very failure mode this PR documents, #3059 / #3062). The new note mitigates this, but detecting the skip (e.g. comparing an app-start timestamp before/after) would make the CLI's success message honest. Might be worth a small issue.
…ApplicationStart Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Wheels Bot — Docs updatedAdded a doc commit to this PR:
|
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR: Re-review at head 45af9d9 — the new commit extends the (already-reviewed) CLI message fix with the four guide pages that previously carried the same false "reload doesn't re-fire onApplicationStart" claim (install.mdx, packages.mdx, 06-authentication.mdx, 08-bonus-basecoat.mdx). I re-verified the corrected contract against the framework source and every rewritten passage is accurate; spec assertions still hold against head. Verdict: comment — no blocking findings, three non-blocking docs/test nits below.
Verification performed
- The corrected contract remains true at this head.
public/Application.cfc:273-296gates?reload=trueon the configuredreloadPassword(constant-time$secureCompare) and routes to$handleRestartAppRequest(), which callsapplicationStop()atpublic/Application.cfc:473— so the next request re-firesonApplicationStartin full. When a configured password is missing/wrong, the gate condition is simply false and the request is served normally: the "silently skips the restart" caveat repeated across the guide pages is exact. - Guide pages checked individually. All four rewritten passages (e.g.
digging-deeper/packages.mdx"The reload re-runsPackageLoader…",06-authentication.mdx"An authorizedwheels reloadre-firesonApplicationStartin full…") state the verified behavior, and each keepswheels stop && wheels startas a valid alternative rather than deleting it. Theinstall.mdxexample output now matches the actual$doInstallreturn string (cli/lucli/services/packages/PackagesMainCli.cfc:219) character-for-character — it didn't even before this PR. - No stale claims left behind in the changed surface. A sweep of
web/sites/guides/src/content/docs/and.ai/for "does not re-fire" / "reload is not enough" / "only fires on a full server boot" phrasings comes back clean.deployment/security-hardening.mdx:175still mentionswheels stop && wheels startbut its claim (a full restart bypasses the HTTP reload endpoint) is accurate and unrelated. - Specs verified against head.
cli/lucli/Module.cfccontains "re-fires onApplicationStart" (lines 843, 849) and contains the banned phrase nowhere; the newPackagesMainCliSpeccase asserts on$doInstall's return string, which includeswheels reloadand not the cold-restart phrase. The new spec reuses the file's established$scratch/$buildStackharness verbatim.
Tests
- Non-blocking, carried over from the previous review —
cli/lucli/tests/specs/commands/ReloadCommandSpec.cfc:24:expect(moduleSource).toInclude("re-fires onApplicationStart")is satisfied by the code comment atModule.cfc:843as well as the user-facingout()atModule.cfc:849, so deleting theout()line while keeping the comment would still pass. Anchoring on the output string (toInclude("Note: an authorized reload re-fires onApplicationStart")) would close that gap. The head commit didn't touch this spec, so the nit stands as-is.
Docs
- Non-blocking —
web/sites/guides/src/content/docs/v4-0-0/command-line-tools/commands/packages/update.mdx:35: the illustrativewheels packages updatesession still showsRestart the server (\wheels stop && wheels start`) to activate it.— butupdate()routes through the same$doInstallthis PR changed (PackagesMainCli.cfc:191→:219), so the real output is nowRun `wheels reload` (or restart) to activate it.Worth updating in this PR (or thebot-update-docs.yml` pass) so the update page doesn't re-teach the myth the rest of the PR removes. - Non-blocking — PR body is stale: it still says the tutorial passages (
06-authentication.mdx,08-bonus-basecoat.mdx) are "out of scope here and are handled separately bybot-update-docs.yml", but commit45af9d9brought them (plus two more pages) into this PR. Suggest a one-line body edit so the description matches the diff for future readers.
Commits
Both commits are commitlint-clean: fix(cli): … and docs(web/guides): correct reload contract — wheels reload re-fires onApplicationStart (valid types, headers under 100 chars, DCO sign-off matching the committer). No direct CHANGELOG.md edit; the fragment changelog.d/3110-reload-refire-contract.fixed.md follows the <slug>.fixed.md convention.
…mented-reload-true-d Reconciles this bot PR with #3125, which already corrected the same tutorial reload passages on develop with different wording. Resolution: - 06-authentication.mdx (lines ~560/~747): develop's #3125 text wins; the "CLI messages still claim... tracked in #3110" parentheticals are shifted to past tense because this PR ships the CLI fix #3110 tracks. - 08-bonus-basecoat.mdx: develop's #3125 step-3 and UIBUTTON troubleshooting text wins (same past-tense adjustment); the expected-output block and $UIBUILDID entry keep this branch's text so the tutorial matches the new 'Run `wheels reload` (or restart) to activate it.' CLI output. - install.mdx, digging-deeper/packages.mdx, and all cli/ changes apply unchanged (no merged docs wave touched them). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com>
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR: Re-review at head b2f5c2d, a merge of origin/develop (which had landed #3125's overlapping tutorial-reload corrections) into this twice-reviewed branch. The substantive change is unchanged from the last review: the CLI's reload note and the packages add activation hint now state the verified contract (an authorized ?reload=true calls applicationStop(), so the next request re-fires onApplicationStart in full), and the four guide pages carry matching wording. I re-verified the contract, the spec assertions, and the merge resolution against this head — all hold, and the merge introduced no regressions or conflict artifacts. Verdict: comment — no blocking findings.
Verification performed
- The corrected contract remains true at this head.
public/Application.cfc:473callsapplicationStop()on an authorized reload, andvendor/wheels/events/onapplicationstart.cfc:380-385re-includesconfig/services.cfm(plus the environment override) on the subsequent boot, with$loadPackagesinvoked from the same startup event — exactly what the new CLI note (cli/lucli/Module.cfc:849) and$doInstallhint (cli/lucli/services/packages/PackagesMainCli.cfc: "Runwheels reload(or restart) to activate it.") now say. The password caveat ("a missing or wrong reload password silently skips the restart") also still matches the gate inpublic/Application.cfc. - Spec assertions hold at head and discriminate old vs new source.
git grepconfirmsonApplicationStart does NOT re-fireno longer appears anywhere incli/lucli/Module.cfc, whilere-fires onApplicationStartappears atModule.cfc:843/849— soReloadCommandSpec'stoInclude/notToIncludepair passes now and would deterministically fail against the old source. The newPackagesMainCliSpectest asserts on the actual$doInstallreturn value (toInclude("wheels reload"),notToInclude("wheels stop && wheels start")), which the head source satisfies; it mirrors the sibling tests'$scratch/$buildStack/stack.cache.refresh()pattern exactly. - Merge resolution checked. No conflict markers in any touched file. The resolution matches the merge commit's stated plan: develop's #3125 wording wins in
06-authentication.mdxand the step-3 / UIBUTTON passages of08-bonus-basecoat.mdx, with the "tracked in #3110" parentheticals correctly shifted to past tense ("CLI versions predating the #3110 fix… that note was wrong") since this PR ships the fix that #3110 tracks.install.mdx,digging-deeper/packages.mdx, and allcli/changes apply unchanged.
Commits
Both branch commits conform to commitlint.config.js: fix(cli): reload and packages add no longer claim onApplicationStart won't re-fire and docs(web/guides): correct reload contract — wheels reload re-fires onApplicationStart (valid types, subjects under 100 chars, not ALL-CAPS). The merge commit carries a descriptive resolution log and a DCO sign-off.
Docs
Changelog fragment present at changelog.d/3110-reload-refire-contract.fixed.md (valid fixed type, complete bullet — no direct CHANGELOG.md edit). Root CLAUDE.md already states the corrected contract ("Restart or wheels reload after install"), so no further update is owed here.
No Correctness, Cross-engine, or Security findings. The non-blocking source-scan-style test nit from the earlier reviews stands as previously noted; nothing new arises from this merge head.
Signed-off-by: Peter Amiri <petera@pai.com> # Conflicts: # cli/lucli/Module.cfc # cli/lucli/tests/specs/commands/ReloadCommandSpec.cfc
Corrects two CLI messages that asserted the opposite of the framework's actual hot-reload behavior. An authorized
?reload=true&password=...callsapplicationStop(), so the next request re-firesonApplicationStartin full —config/services.cfmand the PackageLoader ($loadPackages) re-run (verified live on Lucee 7 in #3110). Yetcli/lucli/Module.cfcprinted "onApplicationStart does NOT re-fire" andcli/lucli/services/packages/PackagesMainCli.cfctold users to runwheels stop && wheels startto activate an installed package. Both are now corrected, and the reload note surfaces the real caveat: a reload only restarts when its password resolves — a missing or wrong password silently serves the request without restarting (the likely origin of the false belief; related to #3059 / #3062).Tutorial/guide passages making the same claim (
06-authentication.mdx,08-bonus-basecoat.mdx) are out of scope here and are handled separately bybot-update-docs.yml.What changed
cli/lucli/Module.cfc—reload()note rewritten from "onApplicationStart does NOT re-fire … runwheels stop && wheels start" to a description of the actual full-restart behavior plus the password-resolution caveat.cli/lucli/services/packages/PackagesMainCli.cfc—$doInstallactivation line changed from "Runwheels stop && wheels startto activate it." to "Runwheels reload(or restart) to activate it."ReloadCommandSpecnow asserts the corrected note (and that the false claim is gone);PackagesMainCliSpecadds a functional assertion that the install output points towheels reload, not a cold restart.TDD
Both specs were written to assert the corrected contract first, confirmed failing against the old source (the
notToInclude/toIncludeassertions are deterministic given the file contents and the$doInstallreturn string), then made to pass by the implementation.Checklist
ReloadCommandSpec+PackagesMainCliSpec, failing → passingbot-update-docs.ymlbot-update-docs.ymlbot-update-docs.yml(root CLAUDE.md already states the correct contract: "Restart orwheels reloadafter install")changelog.d/3110-reload-refire-contract.fixed.mdFixes #3110