Skip to content

fix(cli): provision project rewrite.config on wheels start for 3.x upgrades - #2643

Merged
bpamiri merged 5 commits into
developfrom
fix/bot-2626-default-rewrite-config-404s-static-assets-in-3-x-a
May 13, 2026
Merged

bpamiri merged 5 commits into
developfrom
fix/bot-2626-default-rewrite-config-404s-static-assets-in-3-x-a

Conversation

@wheels-bot

@wheels-bot wheels-bot Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor

wheels start now drops the working rewrite.config template at the project root when one is missing. LuCLI's bundled-default rewrite.config uses a narrow static-dir allow-list (images|css|js|fonts|assets|static) plus negated RewriteCond chains that Tomcat's RewriteValve doesn't honour — every static asset under 3.x-conventional dirs like /miscellaneous/, /javascripts/, /stylesheets/, /files/ 404s on the first wheels start after a 3.x → 4.0 upgrade. Providing a project-level override (which LuCLI's CatalinaBaseConfigGenerator picks up verbatim) sidesteps the buggy default. New apps already receive this file via wheels new; this PR closes the upgrade-path gap. Existing project rewrite.config files are left untouched.

Fixes #2626

Type of Change

  • Bug fix

Feature Completeness Checklist

  • DCO sign-off — commit ships with Signed-off-by:
  • Tests — failing-then-passing spec at vendor/wheels/tests/specs/cli/RewriteConfigInstallerSpec.cfc (5 specs covering missing file, idempotency, allow-list coverage of 3.x dirs, passthrough rule style, missing-template fallback)
  • Framework Docs — left for bot-update-docs.yml
  • AI Reference Docs — left for bot-update-docs.yml
  • CLAUDE.md — left for bot-update-docs.yml
  • CHANGELOG.md — entry under [Unreleased]
  • Test runner passes — wheels.tests.specs.cli runs all 13 specs Passed locally (Lucee 7.0.1.100, SQLite). Spec was authored failing first (6 errors from cli.lucli.services.RewriteConfigInstaller not found by lucee.core), passed after the service landed and Module.cfc wired $ensureProjectRewriteConfig() into start().

Test Plan

  • Boot a 3.x app under Wheels 4.0 with /miscellaneous/, /javascripts/, /stylesheets/, /files/ directories.
  • Verify a rewrite.config appears at the project root after wheels start.
  • Verify CSS/JS requests under those dirs return 200, not 404.
  • Verify re-running wheels start does not overwrite a user-customized rewrite.config.

Notes

  • Service is isolated at cli/lucli/services/RewriteConfigInstaller.cfc so its behavior can be unit-tested without instantiating the full LuCLI module (mirrors BundleStager's pattern).
  • Wiring lives in Module.cfc::start() via $ensureProjectRewriteConfig(), between $ensureWheelsBundles() and the executeCommand("server", ...) delegation.
  • Auto-downgrade safety net not triggered: the diff doesn't touch security, middleware, migrator, deploy, DI, or cross-engine code paths.

…grades

LuCLI's bundled-default rewrite.config uses a narrow static-dir allow-list
(images|css|js|fonts|assets|static) and negated RewriteCond chains. Tomcat's
RewriteValve doesn't honour the negation, so every static asset under
3.x-conventional dirs like /miscellaneous/, /javascripts/, /stylesheets/,
/files/ 404s on the first wheels start after a 3.x → 4.0 upgrade.

wheels start now drops the working template from
cli/lucli/templates/app/rewrite.config into the project root when one is
missing. New apps already get this file via wheels new; this closes the
upgrade-path gap. Existing project rewrite.config files are left untouched.

Fixes #2626

Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
…n issues

Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
@wheels-bot

wheels-bot Bot commented May 13, 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/upgrading/3x-to-4x.mdx — added "Common issues" bullet for static-asset 404s under 3.x-conventional dirs after upgrade, explaining the auto-provisioned rewrite.config fix
  • web/sites/guides/src/content/docs/v4-0-1-snapshot/upgrading/3x-to-4x.mdx — same entry in the snapshot version

@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 A

TL;DR: This PR closes a real 3.x to 4.x upgrade regression (#2626): wheels start now provisions a project-level rewrite.config when one is missing, sidestepping LuCLI's buggy bundled default that 404s static assets under legacy directory names. The service design is solid and the idempotency contract is correct. However, the new test spec re-introduces a cross-engine anti-pattern that the project fixed in #2614 -- directoryCreate(path, true) crashes on Adobe ColdFusion -- and there is a secondary closure-scope issue that will silently produce undefined-variable errors on some engines. Verdict: changes needed in the test spec before merge.


Cross-engine

directoryCreate(projectRoot, true) -- five occurrences in the spec (lines 32, 54, 76, 111, 141)

// vendor/wheels/tests/specs/cli/RewriteConfigInstallerSpec.cfc:32
directoryCreate(projectRoot, true);

This is the exact pattern that #2614 fixed across BrowserTest.cfc and McpServer.cfc. The regression spec at vendor/wheels/tests/specs/wheelstest/BrowserTestArtifactDirSpec.cfc opens with:

// Regression for #2614: Adobe CF rejects directoryCreate(path, true); routes through File.mkdirs() instead.

And every other recent call site in the codebase uses the Java form:

// vendor/wheels/wheelstest/BrowserTest.cfc:284-285
// Adobe CF rejects directoryCreate(path, true) -- see #2614. Use mkdirs() instead.
var created = createObject("java", "java.io.File").init(arguments.path).mkdirs();

Because projectRoot is always a direct child of getTempDirectory() (parent guaranteed to exist), the simplest fix is to drop the second argument -- single-argument directoryCreate is valid on all engines:

directoryCreate(projectRoot);

This affects all five it() blocks. Because the spec lives in vendor/wheels/tests/specs/ -- the directory the core test runner compiles in its entirety -- this will produce Adobe CF errors across every engine x database combination in the CI matrix.


Conventions

Outer-describe var accessed from inner it closures (lines 26-27, used throughout)

// vendor/wheels/tests/specs/cli/RewriteConfigInstallerSpec.cfc:26-27
var repoRoot = expandPath("/wheels/../..");
var templatePath = repoRoot & "/cli/lucli/templates/app/rewrite.config";

var inside a closure writes to that closure's local scope. Per CLAUDE.md "Closure gotcha":

CFML closures can't access outer local vars -- use shared structs

The inner it() callbacks (e.g. line 38: installer.install(..., sourceTemplate=templatePath)) reference templatePath from the outer describe() local scope. On Lucee 7 this works today because arrow-function closures capture the enclosing variable scope; on Adobe CF the lookup can fail with "variable TEMPLATEPATH is undefined". The established fix is a shared struct:

describe("cli.lucli.services.RewriteConfigInstaller", () => {
    var ctx = {};
    ctx.repoRoot     = expandPath("/wheels/../..");
    ctx.templatePath = ctx.repoRoot & "/cli/lucli/templates/app/rewrite.config";

    it("writes...", () => {
        var result = installer.install(projectRoot=projectRoot, sourceTemplate=ctx.templatePath);
    });
});

What is correct

  • The service isolation (mirrors BundleStager) is the right architecture for testability.
  • Idempotency is correctly gated on fileExists(dest) -- re-running wheels start leaves a user-customized file untouched.
  • The template covers all 3.x conventional static dirs (stylesheets, javascripts, files, miscellaneous) and uses positive-match [L]-flagged passthrough rules rather than the negated RewriteCond chains that Tomcat's RewriteValve ignores.
  • $ensureProjectRewriteConfig() is called before executeCommand("server", ...) -- the file lands before Tomcat reads it.
  • CHANGELOG entry is present; DCO Signed-off-by is present; commit message conforms to commitlint.
  • The ## escape in the spec's regex string (reFind("^\s*##", line) becomes runtime regex ^\s*#) is intentional and correct.

Suggested fix (tests only -- no service changes needed)

Replace every directoryCreate(projectRoot, true); with directoryCreate(projectRoot); (5 lines, one per it() block) and wrap repoRoot/templatePath in a ctx struct as shown above. The service CFC and Module.cfc wiring are clean as-is.

@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 A

TL;DR: The new commit (7e5b59256) adds docs-only changes (upgrade guide entries for both v4-0-0 and v4-0-1-snapshot) and is clean. However, the two cross-engine issues flagged in the prior Reviewer A pass at d9e2c9b6b are still unaddressed in vendor/wheels/tests/specs/cli/RewriteConfigInstallerSpec.cfc. Changes needed in the test spec before this can merge.


Cross-engine

directoryCreate(projectRoot, true) — five occurrences (lines 32, 54, 76, 111, 141)

// vendor/wheels/tests/specs/cli/RewriteConfigInstallerSpec.cfc:32
directoryCreate(projectRoot, true);

Adobe ColdFusion does not support the two-argument form of directoryCreate. This is the same pattern that PR #2614 fixed across BrowserTest.cfc and McpServer.cfc, where every call site was replaced with the Java mkdirs() idiom:

// vendor/wheels/wheelstest/BrowserTest.cfc:284-285
// Adobe CF rejects directoryCreate(path, true) -- see #2614. Use mkdirs() instead.
var created = createObject("java", "java.io.File").init(arguments.path).mkdirs();

Because projectRoot is always a direct child of the already-existing getTempDirectory(), the simplest cross-engine fix is the single-argument form (no recursive creation needed):

directoryCreate(projectRoot);

This affects all five it() blocks. The spec lives in vendor/wheels/tests/specs/, which the core test runner compiles in its entirety — one compilation error on Adobe CF crashes every spec in the directory for that engine x database combination.


Conventions

Outer-describe var accessed from inner it closures (lines 26-27, used throughout)

// vendor/wheels/tests/specs/cli/RewriteConfigInstallerSpec.cfc:26-27
var repoRoot = expandPath("/wheels/../..");
var templatePath = repoRoot & "/cli/lucli/templates/app/rewrite.config";

These are used inside every it() block (e.g. line 38: sourceTemplate=templatePath). Per CLAUDE.md "Closure gotcha": CFML closures can't access outer local vars — use shared structs. On Adobe CF the inner it() closures can fail with "variable TEMPLATEPATH is undefined" because var declarations write to the declaring closure's local scope and are not guaranteed to be captured by nested closures on all engines. The established fix is a shared struct:

describe("cli.lucli.services.RewriteConfigInstaller", () => {
    var ctx = {};
    ctx.repoRoot     = expandPath("/wheels/../..");
    ctx.templatePath = ctx.repoRoot & "/cli/lucli/templates/app/rewrite.config";

    it("writes...", () => {
        var result = installer.install(projectRoot=projectRoot, sourceTemplate=ctx.templatePath);
    });
});

What is correct

The new docs commit (7e5b59256) is clean:

  • Both v4-0-0/upgrading/3x-to-4x.mdx and v4-0-1-snapshot/upgrading/3x-to-4x.mdx received the matching entry; no versioned docs directory was missed.
  • The entry accurately describes the symptom, root cause, fix, and idempotency guarantee.
  • Commit conforms to commitlint (docs(web/guides):, <=100 chars, DCO Signed-off-by present).

The service (RewriteConfigInstaller.cfc) and Module.cfc wiring remain correct as noted in the previous pass — no new issues introduced there.


Required fixes (spec only — no service changes needed)

  1. Replace every directoryCreate(projectRoot, true); with directoryCreate(projectRoot); (5 lines, one per it() block).
  2. Wrap repoRoot/templatePath in a ctx struct at the top of the describe() callback and reference them as ctx.templatePath in each it().

@wheels-bot

wheels-bot Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor Author

Wheels Bot — Reviewer B (round 1)

A's review is substantively sound. Both findings are real, cite the correct evidence, and map to established cross-engine gotchas documented in CLAUDE.md and the repo's own changelog. I agree changes are needed in the test spec before merge.

Sycophancy

None detected. The "What is correct" section praises specific, verifiable properties of the diff (idempotency gate on fileExists, call ordering before executeCommand, CHANGELOG presence, DCO trailer) — not generic approval.

False positives

None detected.

  • directoryCreate(projectRoot, true) (Finding 1): A's claim is accurate. The CHANGELOG for Core tests fail on Adobe ColdFusion: DirectoryCreate() called with unsupported arguments in BrowserTest.cfc #2614 records exactly this fix ("Replace Lucee-only directoryCreate(path, true) calls…"). The spec has five occurrences at the lines A cited. A's proposed single-arg workaround is sound: getTempDirectory() always exists so recursive parent creation is unnecessary, and single-arg directoryCreate is valid on all engines.
  • Outer var accessed in inner closures (Finding 2): A quotes the CLAUDE.md "Closure gotcha" rule verbatim and applies it correctly. var repoRoot and var templatePath are declared in the outer describe() callback's local scope and referenced by each of the five inner it() callbacks. The ctx struct pattern A suggests is the established fix in this codebase.

Missed issues

None of significance.

  • directoryDelete(projectRoot, true) appears in every finally block and was not flagged. This is correct — Adobe CF accepts two arguments for directoryDelete; the single-argument validator applies only to directoryCreate. No issue here.
  • The ## escape in the reFind call (line ~123) was correctly noted as intentional and correct. Nothing was missed.

Verdict alignment

Minor process concern: A's TL;DR says "changes needed… before merge" but the review was submitted with state COMMENTED, not CHANGES_REQUESTED. A COMMENTED review does not formally block the PR on GitHub. The substantive recommendation is correct; only the submission state is off. This does not change the convergence decision.

Convergence

Aligned. Both findings are real cross-engine regressions confined to the test spec. The service CFC (RewriteConfigInstaller.cfc) and Module.cfc wiring are clean and need no changes. The required fixes are bounded and unambiguous:

  1. Replace directoryCreate(projectRoot, true) with directoryCreate(projectRoot) at the five call sites (one per it() block).
  2. Replace the bare var repoRoot/var templatePath declarations in the outer describe() callback with a ctx struct: var ctx = { repoRoot: expandPath("/wheels/../..") }; ctx.templatePath = ctx.repoRoot & "/cli/lucli/templates/app/rewrite.config"; and update all inner references to ctx.templatePath.

Joint recommendation: apply these spec-only changes.

@wheels-bot

wheels-bot Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor Author

Wheels Bot — Reviewer B (round 1)

A's review is accurate. Both cross-engine findings are confirmed against the actual spec lines, and there are no false positives or missed issues worth raising. The two changes are concrete and addressable.

Sycophancy

None detected. A did not approve despite unresolved cross-engine issues, and explicitly held the "changes needed" line.

False positives

None detected.

  • directoryCreate(projectRoot, true): A correctly identifies this as Lucee-only. CLAUDE.md explicitly documents this gotcha (directoryCreate(path, true) was fixed in Core tests fail on Adobe ColdFusion: DirectoryCreate() called with unsupported arguments in BrowserTest.cfc #2614 with java.io.File.mkdirs()). BrowserTest.cfc line 280 carries the canonical comment confirming the restriction. A's simpler fix -- drop to single-arg directoryCreate(projectRoot) -- is correct here because getTempDirectory() always returns an existing directory and projectRoot is a direct child, so no recursive creation is needed. This is a verified, real issue at lines 32, 54, 76, 111, and 141.

  • Outer-describe var captured in inner it closures: A correctly cites CLAUDE.md's documented closure gotcha: var declarations in a closure's local scope are not reliably captured by nested closures on Adobe CF. Lines 26-27 declare var repoRoot and var templatePath inside the outer describe() closure; every it() block references templatePath. The ctx struct pattern is the established fix.

Missed issues

None detected. The service code (RewriteConfigInstaller.cfc) is cross-engine clean -- only standard built-in functions with no engine-specific APIs. The Module.cfc wiring uses named arguments and private access correctly. The docs commits are clean. Commit messages and DCO are in order.

Verdict alignment

A submitted as COMMENTED rather than REQUEST_CHANGES, but the body is unambiguous that changes are required before merge. The substance is correct; the formal GitHub verdict is soft. Not a blocking concern given the body is clear.

Convergence

Aligned with A. Both findings are real cross-engine issues that will fail the Adobe CF test run, and the required fixes are precise: (1) replace all five directoryCreate(projectRoot, true) calls with directoryCreate(projectRoot), (2) move repoRoot/templatePath into a ctx struct at the top of the describe() callback and reference them as ctx.templatePath / ctx.repoRoot inside each it(). Changes needed; triggering address-review.

- Replace `directoryCreate(projectRoot, true)` with `directoryCreate(projectRoot)`
  at the five `it()` blocks. Adobe CF rejects the two-arg recursive form
  (per #2614); since `getTempDirectory()` always exists, no recursive parent
  creation is needed.
- Hoist `repoRoot` and `templatePath` from outer-`describe` `var` declarations
  into a shared `ctx` struct so inner `it()` closures see them reliably on
  Adobe CF (CLAUDE.md "Closure gotcha").

No service or wiring changes — service CFC and `Module.cfc::start()` wiring
were already approved.

Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
@wheels-bot

wheels-bot Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor Author

Wheels Bot — Address Review (round 1)

Applied consensus findings from Reviewer A and Reviewer B's convergence (round 1) — both reviewers aligned on two cross-engine fixes confined to the test spec:

  • directoryCreate(projectRoot, true) → directoryCreate(projectRoot) at five sites in vendor/wheels/tests/specs/cli/RewriteConfigInstallerSpec.cfc (the it() blocks at the former lines 32, 54, 76, 111, 141). The two-arg recursive form crashes on Adobe CF (regression covered by Core tests fail on Adobe ColdFusion: DirectoryCreate() called with unsupported arguments in BrowserTest.cfc #2614); getTempDirectory() is always present, so no recursive parent creation is needed.
  • Hoisted repoRoot / templatePath into a shared ctx struct at the top of the outer describe() callback (vendor/wheels/tests/specs/cli/RewriteConfigInstallerSpec.cfc:29-31). The five inner it() callbacks now reference ctx.templatePath rather than the outer-scope var (CLAUDE.md "Closure gotcha" — outer local vars aren't reliably captured by nested closures on Adobe CF).

No service or wiring changes — RewriteConfigInstaller.cfc and Module.cfc::start() were already approved by both reviewers.

Test run: bash tools/test-local.sh cli was not runnable in this environment (the Wheels CLI isn't installed on the runner that authored this commit, and the helper script has a macOS-only sed -i '' invocation that exits before reaching the test loop on Linux). The changes are mechanical — directoryCreate(projectRoot) is the single-argument form already used elsewhere in the codebase, and the ctx-struct pattern matches vendor/wheels/tests/specs/cli/BundleStagerSpec.cfc and the established convention. CI's compat-matrix.yml (Adobe CF 2023 / 2025) will be the authoritative cross-engine signal on the new SHA.

The new commit will trigger a fresh Reviewer A run on the updated SHA. Convergence loop continues until reviewers align on approve or the outer-loop cap (5 rounds) is reached.

@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 A

TL;DR: Commit 5b656e4 addresses both cross-engine issues flagged in the two prior Reviewer A passes. The directoryCreate(projectRoot, true) → directoryCreate(projectRoot) fix is applied at all five it() call sites, and the repoRoot/templatePath variables are correctly hoisted into a shared ctx struct. No new issues introduced. The service CFC, Module.cfc wiring, CHANGELOG entry, and web docs were already approved in earlier rounds. No further changes required — this PR is ready to graduate out of draft.


What was fixed

Cross-engine: directoryCreate(projectRoot, true) → directoryCreate(projectRoot)

All five occurrences updated as requested (lines 36, 58, 80, 115, 145 of RewriteConfigInstallerSpec.cfc):

// Before (Adobe CF rejects the two-arg recursive form — per #2614)
directoryCreate(projectRoot, true);

// After (single-arg, cross-engine safe; parent dir is getTempDirectory() which always exists)
directoryCreate(projectRoot);

Conventions: outer-describe variables shared via ctx struct

Lines 29–31 now match the CLAUDE.md-recommended pattern:

var ctx = {};
ctx.repoRoot     = expandPath("/wheels/../..");
ctx.templatePath = ctx.repoRoot & "/cli/lucli/templates/app/rewrite.config";

Every it() closure references ctx.templatePath rather than a bare outer var templatePath, eliminating the Adobe CF closure-capture risk documented in CLAUDE.md ("Closure gotcha").


Remaining spec details confirmed correct

  • reFind("^\s*##", line) (line 126) — ## correctly escapes to a single # at CFML runtime; the Java regex ^\s*# matches Tomcat rewrite.config comment lines as intended.
  • fileWrite(..., "## user-customized rules") (line 60) and toBe("## user-customized rules") (line 69) — both string literals evaluate to the same runtime value (# user-customized rules); the assertion is internally consistent.
  • directoryDelete(projectRoot, true) in all five finally blocks — the two-arg recursive form for deletion is cross-engine safe; the #2614 fix was scoped to directoryCreate.
  • Commit fix(test): address Reviewer A/B consensus findings (round 1) — type fix, scope test, subject ≤ 100 chars, sentence-case, DCO Signed-off-by present. Conforms to commitlint.config.js.

@wheels-bot

wheels-bot Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor Author

Wheels Bot -- Reviewer B (round 1)

A's approval of the round-1 address-review commit is well-founded. Both prior findings are verifiably fixed, no new regressions were introduced, and the three confirmed-correct details A re-examined all check out against the diff. One minor concern is noted below but it is not blocking.

Sycophancy

None detected. A's sign-off is evidence-based: specific line numbers are cited for each of the five directoryCreate call sites, the before/after snippet is shown, the ctx struct pattern at lines 29-31 is confirmed, and three secondary details are re-checked. This is earned approval, not a rubber stamp.

False positives

None detected.

  • The ## escape analysis for reFind, fileWrite, and toBe is correct. CFML evaluates ## in a string literal to a single hash character at runtime, so the regex passed to reFind is a start-of-line hash pattern, and the two "## user-customized rules" literals both resolve to "# user-customized rules" at runtime -- the fileWrite and toBe values are identical and the assertion is internally consistent.
  • directoryDelete(projectRoot, true): A's scoping note is accurate; the fix in issue 2614 was scoped to directoryCreate only, so the two-arg recursive deletion form is cross-engine safe.

Missed issues

One low-severity item: the variables.moduleRoot trailing-slash assumption. Module.cfc's $ensureProjectRewriteConfig() builds the template path by concatenating variables.moduleRoot with "templates/app/rewrite.config". This works only if variables.moduleRoot ends with a path separator. LuCLI module roots typically do (set by BaseModule init), but A did not verify this. If a given engine omits the trailing slash the installer's fileExists guard returns a silent no-op rather than a crash -- annoying, not catastrophic. Worth a single line of confirmation from a human reviewer, not necessarily a code change.

No other missed issues. The dotted-path resolution of cli.lucli.services.RewriteConfigInstaller in the test, the template file's pre-existence in the repo (added by wheels new), and the ctx struct pattern's correctness all check out.

Verdict alignment

A recommends graduating the PR out of draft with no further code changes; B agrees.

Convergence

Aligned. The two cross-engine issues from the prior round are resolved. The service CFC, Module.cfc wiring, CHANGELOG entry, and web docs are clean. The trailing-slash observation is informational and does not change the recommendation. Both reviewers are satisfied: the PR can be merged once a human approver reviews and flips the draft flag.

Brings the CI-rendered blog visual-regression baseline (8fb7bf2,
landed on develop via #2624's squash merge) into the PR branch so the
Visual regression check stops failing on the stale macOS-Chromium
baseline this branch forked from. The CI run on 5b656e4 measured
37,668 pixels of font-rendering drift against the old baseline; merging
develop in auto-takes the CI-rendered blob (af41788) the PR branch
never touched.

Three conflicts resolved manually, all in the documentation layer:

- CHANGELOG.md: combine #2626 entry (this PR) with develop's #2630,
  #2635, #2621, plus the Documentation subsection for #2627. PR entry
  moved to the end of the Fixed list to preserve develop's ordering.
- web/sites/guides/.../v4-0-0/upgrading/3x-to-4x.mdx: keep both
  Common-issues bullets — develop's wirebox boot-failure entry (#2627)
  and this PR's static-asset 404 entry (#2626).
- web/sites/guides/.../v4-0-1-snapshot/upgrading/3x-to-4x.mdx: same
  resolution as v4-0-0.

cli/lucli/Module.cfc auto-merged cleanly — develop's #2624 added a
sibling helper, this PR added \$ensureProjectRewriteConfig(); both
land side by side.

Signed-off-by: Peter Amiri <peter@alurium.com>
@bpamiri

bpamiri commented May 13, 2026

Copy link
Copy Markdown
Collaborator

Develop catch-up to clear stale visual-regression baseline

Merged develop into the PR branch (commit 40d70f3c) so the Visual regression check picks up the CI-rendered web/tests/visual-baselines/blog.png blob (af41788d) that landed on develop after this PR was opened.

Why the VR check was failing

  • This PR's CI ran at 2026-05-13T02:39Z against merge-base 12e82e50, which carries the old macOS-Chromium baseline blob f0a9e797.
  • The CI-rendered baseline replacement (8fb7bf26) shipped to develop ~80 minutes later via fix(cli): honor --no-sqlite when scaffolding lucee.json #2624's squash merge (1e7dfa49 at 04:03Z).
  • The 37,668-pixel diff was macOS-vs-Linux font hinting drift against the stale baseline, not anything in this PR. Reviewer A and Reviewer B were already converged on approve for 5b656e46.

Conflicts resolved manually (docs-only)

  • CHANGELOG.md — Combined this PR's #2626 entry with develop's #2630 / #2635 / #2621 Fixed entries and the ### Documentation subsection from #2627. The #2626 entry moved to the end of the Fixed list to preserve develop's ordering.
  • web/sites/guides/src/content/docs/v4-0-0/upgrading/3x-to-4x.mdx — Kept both Common-issues bullets side-by-side: develop's wirebox boot-failure entry (#2627) and this PR's static-asset 404 entry (#2626).
  • web/sites/guides/src/content/docs/v4-0-1-snapshot/upgrading/3x-to-4x.mdx — Same resolution as v4-0-0.

cli/lucli/Module.cfc auto-merged cleanly — develop's #2624 added a sibling helper; this PR's $ensureProjectRewriteConfig() lands beside it. No service or test changes.

Reviewer A will re-run on the new SHA. The merge commit is pure conflict resolution and develop catch-up, so the prior approve convergence should hold.

@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.

(Submitted in error — content redacted)

@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 A

TL;DR: Pass-through review of merge commit (40d70f3ce) integrating develop after round 3. No new issues in PR code. Pre-existing closure-scope issue in ConfigRoutesStaleDocUrlSpec.cfc arrived via develop (PR #2635) — not introduced by this PR. This PR is ready to graduate from draft.


Cross-engine

Pre-existing: outer-describe var repoRoot referenced inside nested it() closures — ConfigRoutesStaleDocUrlSpec.cfc (line 41)

The pattern var repoRoot = expandPath("/wheels/../..") is declared in outer describe() and referenced inside it() closures. On Adobe CF this fails with variable REPOROOT is undefined because CFML closures do not reliably capture outer local vars (CLAUDE.md “Closure gotcha”).

This file came from develop via the merge commit — not introduced by this PR. Landed on develop as part of PR #2635; track as separate follow-up against develop. This PR’s RewriteConfigInstallerSpec.cfc correctly avoids the pattern using ctx.repoRoot.


What the merge confirmed is still correct

  • RewriteConfigInstaller.cfc — arguments. prefix throughout; idempotency on fileExists(dest); all five return-struct cases; mirrors BundleStager.
  • Module.cfc:4507-4519 — $ensureProjectRewriteConfig() is private void, called before executeCommand("server", ...), swallows IO failures silently.
  • RewriteConfigInstallerSpec.cfc — directoryCreate(projectRoot) single-arg; ctx struct; directoryDelete(projectRoot, true) in all five finally blocks.
  • rewrite.config template — covers miscellaneous, stylesheets, javascripts, files; positive-match [L]-flagged rules; no negated RewriteCond.
  • CHANGELOG and both v4-0-0 / v4-0-1-snapshot upgrade guides updated.

Commits

SHA Message Verdict
d9e2c9b6b fix(cli): provision project rewrite.config… pass
7e5b59256 docs(web/guides): note static-asset 404 fix… pass
5b656e462 fix(test): address Reviewer A/B consensus findings (round 1) pass
40d70f3ce merge commit n/a

@wheels-bot

wheels-bot Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor Author

Wheels Bot — Reviewer B (round 1)

Review #4278722945 (and the second submission #4278728593) on the current SHA were both self-retracted by Reviewer A with the note "Submitted in error — content redacted." Neither carries any findings, verdict, or line comments to critique.

Sycophancy

N/A — no review body to assess.

False positives

N/A — no findings were made.

Missed issues

N/A — no content was submitted.

Verdict alignment

No verdict was issued.

Convergence

Not aligned — not because A and B disagree, but because no valid Reviewer A review exists for this SHA (40d70f3c). The prior convergence (converged-approve on 5b656e46) covered the spec-only cross-engine fixes; the current SHA is a pure conflict-resolution merge of develop with no functional changes (confirmed in the bpamiri catch-up comment above). A valid Reviewer A submission is needed on this SHA before convergence can be declared.

@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 A response (round 1)

B's round-1 critique was factually correct at the moment it was posted: the two prior A submissions on SHA 40d70f3ce were both self-retracted (content redacted), leaving no valid Reviewer A review for that SHA. However, A's substantive review (PRR_kwDOABMUH87_CDeR) was submitted at 05:55:27Z — approximately 35 seconds before B's comment at 05:56:02Z. The valid review existed; B's trigger fired before it was indexed. No substantive disagreement separates A and B.

Conceded points

  • B was correct that at the time of the round-1 critique, no valid A review could be observed on this SHA. The timing race is acknowledged.

Defended points

  • A's valid review (PRR_kwDOABMUH87_CDeR) on SHA 40d70f3ce is the authoritative A position. It confirmed: (1) RewriteConfigInstallerSpec.cfc correctly uses single-arg directoryCreate(projectRoot) at all five it() call sites; (2) the ctx.repoRoot / ctx.templatePath struct pattern is applied throughout the outer describe() callback; (3) the service CFC, Module.cfc wiring, CHANGELOG, and both upgrade guide docs are clean. One pre-existing closure-scope issue in ConfigRoutesStaleDocUrlSpec.cfc (outer var repoRoot captured by nested it() closures) was noted — that file arrived via develop/PR #2635, is not part of this PR, and should be tracked as a separate follow-up against develop.
  • The recommendation to graduate from draft is based on verified evidence in the diff, not solely on the prior rounds' convergence.

Updated verdict

Still recommend graduating from draft (approve). All PR-specific cross-engine issues are resolved; no new issues were introduced by the merge commit. The one noted issue (ConfigRoutesStaleDocUrlSpec.cfc) is a pre-existing develop regression and does not block this PR.

@bpamiri
bpamiri marked this pull request as ready for review May 13, 2026 06:42
Second develop catch-up — five more PRs landed on develop after the
first merge (40d70f3): #2637, #2645, #2639, #2638, #2642. Re-resolves
the same three documentation conflicts:

- CHANGELOG.md: keep develop's #2628 and #2629 Fixed entries; move this
  PR's #2626 entry to the end of the Fixed list.
- v4-0-0/upgrading/3x-to-4x.mdx: keep develop's new #2631 reloadPassword
  Common-issues bullet; this PR's #2626 static-asset bullet stays at
  the end.
- v4-0-1-snapshot/upgrading/3x-to-4x.mdx: same resolution.

cli/lucli/Module.cfc auto-merged again — no manual fixes needed.

Signed-off-by: Peter Amiri <peter@alurium.com>
@bpamiri
bpamiri merged commit bac76bd into develop May 13, 2026
12 of 13 checks passed
@bpamiri
bpamiri deleted the fix/bot-2626-default-rewrite-config-404s-static-assets-in-3-x-a branch May 13, 2026 06:50

@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 A

TL;DR: Third Reviewer A pass against head SHA 7b9c97f47, a second merge commit integrating develop into fix/bot-2626. No new issues were introduced by this integration. All cross-engine and conventions findings from prior rounds remain resolved. The PR-specific files (RewriteConfigInstaller.cfc, RewriteConfigInstallerSpec.cfc, Module.cfc, docs, CHANGELOG) are clean. Verdict: approve (blocked by self-review restriction — bot authored this PR; a human reviewer should merge).


What this pass confirms

Service code: cli/lucli/services/RewriteConfigInstaller.cfc

  • All five return paths are correctly modelled: missing-project-root, already-present, missing-template, write-failed, and the success path.
  • Idempotency is correctly gated on fileExists(dest) before any write, so user-customised files are left untouched.
  • arguments. prefix is used throughout the function body — no naked variable references.
  • The write-failed path is caught locally (in install()) and returns a structured result rather than throwing, while $ensureProjectRewriteConfig() in Module.cfc wraps the entire call in a second try/catch as a belt-and-suspenders silent guard so a permissions hiccup never blocks wheels start.

Spec: vendor/wheels/tests/specs/cli/RewriteConfigInstallerSpec.cfc

  • directoryCreate(projectRoot) — single-arg form used at all five it() call sites (lines 36, 58, 80, 115, 145). Cross-engine safe per #2614.
  • ctx.repoRoot / ctx.templatePath — outer-describe values shared via a struct rather than bare outer var declarations, matching the CLAUDE.md closure-gotcha pattern.
  • directoryDelete(projectRoot, true) in all five finally blocks — the two-arg recursive form for deletion is cross-engine safe (the #2614 fix was scoped to directoryCreate).
  • reFind("^\s*##", line) (line 126) — CFML evaluates ## to a single # in string literals, so the runtime regex is ^\s*#, which correctly matches Tomcat RewriteValve comment lines. Intentional and correct.
  • fileWrite(... "## user-customized rules") and toBe("## user-customized rules") — both sides evaluate to # user-customized rules at runtime via the same CFML ## escape; the idempotency assertion is internally consistent.
  • reFind("RewriteCond\s+%\{REQUEST_URI\}\s+!", activeRules) > 0 (line 131) — no # characters in the pattern; the \s, \{, \} sequences are CFML-safe and correctly pass through to the Java regex engine.

Module.cfc wiring

  • $ensureProjectRewriteConfig() is declared private void, placed between $ensureWheelsBundles() and the executeCommand("server", ...) delegation — correct position so the file lands before Tomcat reads it.

Docs and metadata

  • CHANGELOG [Unreleased] entry is present, accurate, and describes both symptom and fix.
  • Both v4-0-0/upgrading/3x-to-4x.mdx and v4-0-1-snapshot/upgrading/3x-to-4x.mdx received the matching entry.

Commits (PR-specific)

SHA Message Verdict
d9e2c9b6b fix(cli): provision project rewrite.config on wheels start for 3.x upgrades pass
7e5b59256 docs(web/guides): note static-asset 404 fix for 3.x upgrades in common issues pass
5b656e462 fix(test): address Reviewer A/B consensus findings (round 1) pass
7b9c97f47 merge commit n/a

Pre-existing issue (not from this PR — for tracking only)

vendor/wheels/tests/specs/cli/ConfigRoutesStaleDocUrlSpec.cfc (arrived via develop / PR #2635) still declares var repoRoot = expandPath("/wheels/../.."); in an outer describe() and references it inside nested it() closures. This is the closure-capture gotcha that would fail on Adobe CF. Flagged in the prior round-4 pass; should be tracked as a separate follow-up against develop, not as a blocker here.

@wheels-bot

wheels-bot Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor Author

Wheels Bot — Reviewer B (round 1)

A's approval of the second develop-catchup merge commit is well-founded and accurate. Every claim A made against the diff checks out. No sycophancy, no false positives, no missed issues of consequence. Aligned.

Sycophancy

None detected. A's sign-off cites specific line numbers for each of the five directoryCreate call sites, confirms the ctx struct pattern at the outer-describe level, re-examines three secondary escape/regex details, and explicitly notes the self-review restriction rather than rubber-stamping. This is earned approval.

False positives

None detected.

  • directoryCreate(projectRoot) (lines 36, 58, 80, 115, 145): Confirmed against the diff — single-arg form at all five sites.
  • ctx.repoRoot / ctx.templatePath struct pattern: Confirmed at lines 29-31 of the spec. The outer var ctx = {} and two ctx.* assignments sit in the describe() callback; every it() block references ctx.templatePath. Matches the CLAUDE.md closure-gotcha pattern.
  • reFind("^\s*##", line): A's escape analysis is accurate. CFML evaluates ## to a single # in string literals, so the Java regex engine receives ^\s*#, correctly matching RewriteValve comment lines.
  • fileWrite / toBe with "## user-customized rules": Both sides evaluate to "# user-customized rules" at runtime; the idempotency assertion is internally consistent.
  • directoryDelete(projectRoot, true) (two-arg form): A's scoping note is correct — the fix in issue 2614 applied only to directoryCreate. Two-arg recursive deletion is cross-engine safe.

Missed issues

None of substance.

  • ##2626 in the spec's block comment header (See issue ##2626.): In a /** */ comment block, CFML does not evaluate expressions, so ##2626 renders literally as two hash characters rather than one. Cosmetically odd but causes no compilation error or test failure. Not a blocker.
  • variables.moduleRoot trailing-slash assumption: Raised as an informational observation by prior B rounds and acknowledged as non-blocking. A re-flagged it under "Pre-existing issue." No new code to change.
  • Merge commit content: The conflict-resolution merge commit (7b9c97f47) was pure docs + CHANGELOG — bpamiri's catch-up comment describes every hunk. A's "n/a" verdict is appropriate.

Verdict alignment

A recommends approve with the self-review restriction noted (the bot authored the PR and cannot formally submit a GitHub approval). Submitting as COMMENTED is the correct workaround and consistent with prior rounds. Verdict is aligned with the diff.

Convergence

Aligned. The two cross-engine issues from round 1 are resolved, confirmed across two prior B-round assessments and an address-review commit. This third Reviewer A pass against the develop-catchup SHA finds nothing new. Joint recommendation: approve — the PR is ready for a human reviewer to flip the draft flag and merge.

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.

Default rewrite.config 404s static assets in 3.x app trees (non-/images,/css,/js dirs)

1 participant