Skip to content

fix(cli): reload and packages add no longer claim onApplicationStart won't re-fire#3126

Merged
bpamiri merged 4 commits into
developfrom
fix/bot-3110-hot-reload-contract-is-misdocumented-reload-true-d
Jun 13, 2026
Merged

fix(cli): reload and packages add no longer claim onApplicationStart won't re-fire#3126
bpamiri merged 4 commits into
developfrom
fix/bot-3110-hot-reload-contract-is-misdocumented-reload-true-d

Conversation

@wheels-bot

@wheels-bot wheels-bot Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Corrects two CLI messages that asserted the opposite of the framework's actual hot-reload behavior. An authorized ?reload=true&password=... calls applicationStop(), so the next request re-fires onApplicationStart in fullconfig/services.cfm and the PackageLoader ($loadPackages) re-run (verified live on Lucee 7 in #3110). Yet cli/lucli/Module.cfc printed "onApplicationStart does NOT re-fire" and cli/lucli/services/packages/PackagesMainCli.cfc told users to run wheels stop && wheels start to 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 by bot-update-docs.yml.

What changed

  • cli/lucli/Module.cfcreload() note rewritten from "onApplicationStart does NOT re-fire … run wheels stop && wheels start" to a description of the actual full-restart behavior plus the password-resolution caveat.
  • cli/lucli/services/packages/PackagesMainCli.cfc$doInstall activation line changed from "Run wheels stop && wheels start to activate it." to "Run wheels reload (or restart) to activate it."
  • Specs: ReloadCommandSpec now asserts the corrected note (and that the false claim is gone); PackagesMainCliSpec adds a functional assertion that the install output points to wheels reload, not a cold restart.

TDD

Both specs were written to assert the corrected contract first, confirmed failing against the old source (the notToInclude/toInclude assertions are deterministic given the file contents and the $doInstall return string), then made to pass by the implementation.

Note: the CLI suite (bash tools/test-cli-local.sh, which boots a server via lucli) could not be executed in this sandbox — direct lucli/test-cli-local.sh invocations require an approval that isn't available here. The two assertions are pure source-scan / return-string checks whose pass/fail was verified by inspecting the exact source each spec reads. CI's bot-tdd-gate + CLI test job will execute the suite.

Checklist

  • Tests — ReloadCommandSpec + PackagesMainCliSpec, failing → passing
  • Framework Docs — handled by bot-update-docs.yml
  • AI Reference Docs — handled by bot-update-docs.yml
  • CLAUDE.md — handled by bot-update-docs.yml (root CLAUDE.md already states the correct contract: "Restart or wheels reload after install")
  • CHANGELOG.md — changelog.d/3110-reload-refire-contract.fixed.md
  • Test runner passes — see TDD note above re: sandbox limitation

Fixes #3110

…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>

@wheels-bot wheels-bot Bot left a comment

Copy link
Copy Markdown
Contributor Author

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 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-295 gates ?reload=true on the configured reloadPassword (constant-time compare) and routes to $handleRestartAppRequest(), which calls applicationStop() at public/Application.cfc:473. On the next request, vendor/wheels/events/onapplicationstart.cfc:380-385 re-includes config/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.cfc no longer contains onApplicationStart does NOT re-fire anywhere (the remaining wheels stop && wheels start at Module.cfc:901 is the unrelated stop command hint and is no longer asserted against). $doInstall's return string (PackagesMainCli.cfc:218-219) contains wheels reload and not wheels stop && wheels start; the new code comment splits that phrase across lines 214-215, but PackagesMainCliSpec correctly 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 fragment changelog.d/3110-reload-refire-contract.fixed.md follows the <slug>.fixed.md convention 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 at Module.cfc:843 as well as the user-facing out() call at Module.cfc:849. If a future change deleted the out() 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 the out() line. The source-scan approach itself is fine (it is the pre-existing pattern for this spec, acknowledged in its header), and the notToInclude regression guard at line 26 is solid either way.

Docs

  • Deferring the tutorial/guide passages (06-authentication.mdx, 08-bonus-basecoat.mdx) to bot-update-docs.yml is reasonable, and root CLAUDE.md already states the correct contract for package activation ("Restart or wheels reload after 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

wheels-bot Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Wheels Bot — Docs updated

Added a doc commit to this PR:

  • web/sites/guides/src/content/docs/v4-0-0/command-line-tools/commands/packages/install.mdx — corrected description and example output to show wheels reload activates a package; removed false claim that reload is insufficient
  • web/sites/guides/src/content/docs/v4-0-0/digging-deeper/packages.mdx — updated install snippet and explanation to use wheels reload; added password-resolution caveat
  • web/sites/guides/src/content/docs/v4-0-0/start-here/tutorial/06-authentication.mdx — corrected two passages that claimed wheels reload does not re-fire onApplicationStart
  • web/sites/guides/src/content/docs/v4-0-0/start-here/tutorial/08-bonus-basecoat.mdx — updated expected CLI output, install step, and three troubleshooting items to reflect that wheels reload (with correct password) is sufficient to activate a package

@wheels-bot wheels-bot Bot left a comment

Copy link
Copy Markdown
Contributor Author

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: 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-296 gates ?reload=true on the configured reloadPassword (constant-time $secureCompare) and routes to $handleRestartAppRequest(), which calls applicationStop() at public/Application.cfc:473 — so the next request re-fires onApplicationStart in 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-runs PackageLoader…", 06-authentication.mdx "An authorized wheels reload re-fires onApplicationStart in full…") state the verified behavior, and each keeps wheels stop && wheels start as a valid alternative rather than deleting it. The install.mdx example output now matches the actual $doInstall return 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:175 still mentions wheels stop && wheels start but its claim (a full restart bypasses the HTTP reload endpoint) is accurate and unrelated.
  • Specs verified against head. cli/lucli/Module.cfc contains "re-fires onApplicationStart" (lines 843, 849) and contains the banned phrase nowhere; the new PackagesMainCliSpec case asserts on $doInstall's return string, which includes wheels reload and not the cold-restart phrase. The new spec reuses the file's established $scratch/$buildStack harness 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 at Module.cfc:843 as well as the user-facing out() at Module.cfc:849, so deleting the out() 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 illustrative wheels packages update session still shows Restart the server (\wheels stop && wheels start`) to activate it.— butupdate()routes through the same$doInstall this PR changed (PackagesMainCli.cfc:191:219), so the real output is now Run `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 by bot-update-docs.yml", but commit 45af9d9 brought 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>

@wheels-bot wheels-bot Bot left a comment

Copy link
Copy Markdown
Contributor Author

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: 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:473 calls applicationStop() on an authorized reload, and vendor/wheels/events/onapplicationstart.cfc:380-385 re-includes config/services.cfm (plus the environment override) on the subsequent boot, with $loadPackages invoked from the same startup event — exactly what the new CLI note (cli/lucli/Module.cfc:849) and $doInstall hint (cli/lucli/services/packages/PackagesMainCli.cfc: "Run wheels 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 in public/Application.cfc.
  • Spec assertions hold at head and discriminate old vs new source. git grep confirms onApplicationStart does NOT re-fire no longer appears anywhere in cli/lucli/Module.cfc, while re-fires onApplicationStart appears at Module.cfc:843/849 — so ReloadCommandSpec's toInclude/notToInclude pair passes now and would deterministically fail against the old source. The new PackagesMainCliSpec test asserts on the actual $doInstall return 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.mdx and the step-3 / UIBUTTON passages of 08-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 all cli/ 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.

@bpamiri bpamiri marked this pull request as ready for review June 12, 2026 18:21
@bpamiri bpamiri enabled auto-merge (squash) June 12, 2026 18:26
Signed-off-by: Peter Amiri <petera@pai.com>

# Conflicts:
#	cli/lucli/Module.cfc
#	cli/lucli/tests/specs/commands/ReloadCommandSpec.cfc
@bpamiri bpamiri merged commit 986e68a into develop Jun 13, 2026
18 of 19 checks passed
@bpamiri bpamiri deleted the fix/bot-3110-hot-reload-contract-is-misdocumented-reload-true-d branch June 13, 2026 04:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

1 participant