Skip to content

fix(cli): guard application.wo in onError so init failures don't cascade#2774

Merged
bpamiri merged 4 commits into
developfrom
fix/bot-2773-first-15-minutes-tutorial-fails-the-key-wo-does-no
May 21, 2026
Merged

fix(cli): guard application.wo in onError so init failures don't cascade#2774
bpamiri merged 4 commits into
developfrom
fix/bot-2773-first-15-minutes-tutorial-fails-the-key-wo-does-no

Conversation

@wheels-bot
Copy link
Copy Markdown
Contributor

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

Summary

When the Wheels Injector fails to load during onApplicationStart (a stale /wheels mapping under Lucee Express 7 is what users hit on the "Your First 15 Minutes" tutorial), application.wo is never assigned. The existing recovery try/catch inside onError swallows a second failure silently, then unconditionally calls application.wo.$getRequestTimeout() — which throws The key [WO] does not exist. and replaces the real diagnostic with a cryptic cascade.

This PR adds a StructKeyExists(application, "wo") guard right after the recovery try/catch in cli/lucli/templates/app/public/Application.cfc (the template behind wheels new) and the demo public/Application.cfc. When the global isn't there, the handler renders a minimal HTML error page (heading + initialization message + the original exception's message if available) and returns. The user now sees the underlying init failure instead of the WO cascade.

Note: this is the defensive fix the triage comment called out as worth doing regardless of root cause. It does not chase the deeper wheels new / mapping-resolution question (the triage assessed that as confidence: medium across multiple plausible layers) — that stays open for follow-up investigation via tools/test-onboarding.sh.

Related Issue

Closes #2773

Type of Change

  • Bug fix
  • New feature
  • Enhancement to existing feature
  • Documentation update
  • Refactoring

Feature Completeness Checklist

  • DCO sign-offSigned-off-by: trailer present
  • Testsvendor/wheels/tests/specs/cli/OnErrorFallbackGuardSpec.cfc asserts each Application.cfc (template + demo) guards application.wo with StructKeyExists(application, "wo") after the recovery try/catch, and that the guard appears before the first application.wo.* dereference. The spec fails against the pre-fix templates (no such guard existed in either file) and passes after the implementation lands.
  • Framework Docs — leave unchecked (bot-update-docs.yml will add MDX updates as a follow-up if needed)
  • AI Reference Docs — leave unchecked (handled by bot-update-docs.yml)
  • CLAUDE.md — leave unchecked (handled by bot-update-docs.yml)
  • CHANGELOG.md[Unreleased] entry added under "Fixed"
  • Test runner passes — Local test execution couldn't run inside the bot's sandbox (no wheels CLI on PATH, no Docker available, restricted filesystem); the CI compat-matrix on this PR will execute the spec across every engine × DB.

Test Plan

  • CI compat-matrix runs vendor/wheels/tests/specs/cli/OnErrorFallbackGuardSpec.cfc on every supported engine/DB combination
  • Reviewer manually reproduces with bash tools/test-onboarding.sh on a fresh-install harness to confirm the cascade no longer happens when Injector load fails

Screenshots / Output

Before (reporter's stack trace from #2773):

The Error Occurred in
/home/tim/Projects/hello/public/Application.cfc: line 277
275:       }
276:
277:       local.requestTimeout = application.wo.$getRequestTimeout() + 30;

After (with application.wo missing): a <h1>Application Error</h1> block plus the original exception message — same exception object, surfaced instead of hidden.

When the Wheels Injector fails to load during onApplicationStart (a stale
/wheels mapping under Lucee Express 7 is the symptom users hit on the
"Your First 15 Minutes" tutorial), application.wo is never assigned. The
existing recovery try/catch inside onError swallows a second failure
silently and then unconditionally calls application.wo.$getRequestTimeout(),
which throws "The key [WO] does not exist." and replaces the real
diagnostic with a cryptic cascade.

Add a StructKeyExists(application, "wo") guard right after the recovery
try/catch in cli/lucli/templates/app/public/Application.cfc (the template
behind `wheels new`) and the demo public/Application.cfc. When the global
isn't there, render a minimal HTML error page and return — the user sees
"Wheels failed to initialize" plus the original exception message instead
of the cascade.

Fixes #2773

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

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

wheels-bot Bot commented May 21, 2026

Wheels Bot — Docs updated

Added a doc commit to this PR:

  • web/sites/guides/src/content/docs/v4-0-1-snapshot/start-here/first-15-minutes.mdx — added a caution Aside after "Start the server" explaining what to do if the browser shows the new "Application Error" page (stale /wheels mapping, check server log, run wheels reload)
  • .ai/wheels/troubleshooting/common-errors.md — added "Application Error — Wheels failed to initialize" entry documenting the new error surface, its cause (failed onApplicationStart before application.wo is assigned), and resolution steps

Copy link
Copy Markdown
Contributor Author

@wheels-bot wheels-bot Bot left a comment

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: PR #2774 fixes a cascading failure in onError where a missed Wheels Injector init (e.g. stale /wheels mapping) causes a secondary "The key [WO] does not exist." to replace the real diagnostic. The fix — a StructKeyExists(application, "wo") guard after the recovery try/catch in both Application.cfc files — is correct, minimal, and safe. Two minor nits follow, neither blocking. Verdict: comment.


Correctness

The guard logic in both cli/lucli/templates/app/public/Application.cfc:281-293 and public/Application.cfc:295-307 is placed correctly: it fires after the recovery try/catch closes and before the first application.wo.$getRequestTimeout() dereference at lines 295 and 311 respectively.

The fallback path is safe: encodeForHTML(arguments.Exception.message) prevents XSS from a crafted exception message. The isStruct check on arguments.Exception is a sensible guard given the any type annotation.

Nit — missing HTTP 500 status code. The fallback calls WriteOutput(...) and return with no status code, so the response defaults to HTTP 200. Monitoring tools and CDNs will cache this as a success. Consider adding cfheader(statusCode=500, statusText="Internal Server Error") (or the script equivalent) before the WriteOutput block. It's a one-liner and dramatically improves observability for users who hit this path.

// suggested addition before WriteOutput
local.statusArgs = {statusCode: 500, statusText: "Internal Server Error"};
cfheader(attributeCollection=local.statusArgs);

Note: per CLAUDE.md anti-pattern #10, a plain struct copy is required for attributeCollection on Adobe CF 2023/2025 — the snippet above already follows that pattern.


Tests

vendor/wheels/tests/specs/cli/OnErrorFallbackGuardSpec.cfc is a structural/static-analysis spec: it reads both Application.cfc files, strips comments via $stripCfmlComments, extracts the onError function body with a brace-counter, and asserts that the guard appears before the first application.wo.* dereference. This approach correctly enforces the structural invariant and was the right call (a runtime simulation of a failed Injector init would require Docker + a bad mapping, which is out of scope for the spec suite).

The closure-capture IIFE (function(relPath) {...})(rel) at line 35 correctly avoids the loop-variable-capture issue (CLAUDE.md anti-pattern #10). The ## escaping in all string literals is correct.

Nit — catchClosePattern is fragile. The regex at line 91:

var catchClosePattern = "catch\s*\(\s*any\s+\w+\s*\)\s*\{[^\}]*\}";

The [^\}]* character class only handles catch bodies with no nested braces. This works today because the outer catch body contains only a stripped comment. If someone later adds a conditional or a nested try inside the outer catch, the regex won't match, catchMatch.pos[1] will be 0, scanFrom falls back to 1, and the spec silently scans from the top of onErrorBody instead of after the catch — which still passes since the guard exists. The fragility is tolerable given the fallback, but it's worth a short comment above the pattern explaining the assumption.


Cross-engine

No issues. StructKeyExists, WriteOutput, encodeForHTML, isStruct, and setting requestTimeout=30 are all standard CFML available on Lucee 5/6/7, Adobe CF 2018–2025, and BoxLang. No application scope function members, no bracket-notation calls, no obj.map() collisions.


Commits

fix(cli): guard application.wo in onError so init failures don't cascade

Type fix, scope cli, subject 68 chars, sentence-case — passes commitlint.config.js.


Security

encodeForHTML(arguments.Exception.message) is applied before rendering in the <pre> block (cli/lucli/templates/app/public/Application.cfc:287, public/Application.cfc:301). No SQL, no user-controlled keys stored, no secrets in fixtures. Clean.


Docs

CHANGELOG [Unreleased] entry is present and accurate. PR template is filled honestly (test runner flag explained, bot-update-docs deferred items called out).

Copy link
Copy Markdown
Contributor Author

@wheels-bot wheels-bot Bot left a comment

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 (71c35ec) adds documentation for the application.wo guard introduced in the prior fix commit (ad91b70). Both the .ai/ reference doc and the user-facing guide addition are well-placed. Two documentation nits follow, neither blocking; neither open nit from the prior review has been addressed (not re-escalated). Verdict: comment.


Docs

web/sites/guides/src/content/docs/v4-0-1-snapshot/start-here/first-15-minutes.mdx — contradictory recovery steps

The new <Aside> instructs new users:

try stopping the server, running wheels reload, and restarting

wheels reload sends an HTTP request to the running server (?reload=true&password=...). It cannot run while the server is stopped — the request has nowhere to go. A user following this literally would stop the server, fire wheels reload into the void, then restart. The .ai/wheels/troubleshooting/common-errors.md sibling added in the same commit gives the correct disjunction on its step 4:

Run wheels reload (or stop/start the server) to pick up the corrected mapping.

Suggested replacement for the Aside sentence:

The most common cause on a fresh install is a stale `/wheels` CFML mapping.
While the server is running, try `wheels reload` to pick up the corrected
mapping. If that does not clear it, stop and restart the server. The server
log will point to the specific file that failed to load.

.ai/wheels/troubleshooting/common-errors.md — speculative version reference

The closing Note reads:

If you see the old cascade on a pre-4.0.2 app…

The fix is in [Unreleased] — the next version has not been released and may not be named 4.0.2. Referencing an unreleased version number in a doc shipped with the fix means the note will be wrong if the release is named differently. Safer wording:

If you see the old cascade on a version that predates this fix (i.e. 4.0.1 or earlier)…


Commits

docs(web/guides): note application error fallback and init failure in troubleshooting docs

Type docs, scope web/guides (listed as a suggested scope in CLAUDE.md), subject 73 chars, sentence-case — passes commitlint.config.js. DCO sign-off present.

fix(cli): guard application.wo in onError so init failures don't cascade

Reviewed in prior pass; no change to that assessment.


Open nits from prior review (not re-escalated)

Neither was addressed; both remain tolerable.

  1. HTTP 500 status code missing — the onError fallback in both Application.cfc files returns HTTP 200 when application.wo is absent. Monitoring tools and CDNs treat this as a success response. A cfheader call with statusCode=500 before the WriteOutput block would fix observability. See prior review for the Adobe-safe snippet using a local plain struct.

  2. Fragile catchClosePattern regex[^\}]* in OnErrorFallbackGuardSpec.cfc:91 won't match catch bodies containing nested braces; the silent fallback to scanFrom = 1 masks the degradation. A short comment above the pattern noting the assumption would help future maintainers.

@wheels-bot
Copy link
Copy Markdown
Contributor Author

wheels-bot Bot commented May 21, 2026

Wheels Bot — Reviewer B (round 1)

A's review is thorough and accurate. The two nits are legitimate, the cross-engine check is correct, and the verdict aligns with the findings. One structural note: A reviewed at SHA `ad91b70e1` and a docs commit (`71c35ec80`) landed on top before this critique; the docs changes are clean (accurate `.ai/wheels/troubleshooting/common-errors.md` entry, correct MDX Aside in the guides). No concerns there.

Sycophancy

None detected. A gave a "comment" verdict with two concrete, cited nits rather than an empty LGTM.

False positives

None detected. I verified each claim against the actual diff lines:

  • Guard placement (`cli/lucli/templates/app/public/Application.cfc:281–293`, `public/Application.cfc:295–307`): confirmed correct — after the outer catch closes, before the first `application.wo.*` dereference.
  • `encodeForHTML` + `isStruct` guard: both present and correct in both files.
  • HTTP 500 nit: valid. The fallback returns HTTP 200 for an error page, which misleads monitoring tools and CDNs. A's suggested code uses a plain struct (`local.statusArgs = {statusCode: 500, ...}`) — correctly following CLAUDE.md anti-pattern Fixed bug in $findRoute() that causes blow up on unmatched named route #10 (Adobe CF 2023/2025 reject `arguments` scope as `attributeCollection` on built-in tags). The snippet is sound.
  • Fragile `catchClosePattern` regex: valid. `[^\}]*` matches only catch bodies with no nested braces. Works today because after comment stripping the body is just whitespace; A correctly notes the fallback (`scanFrom = 1`) keeps the spec passing even if the pattern misses.
  • IIFE loop-capture pattern: A cites CLAUDE.md anti-pattern Fixed bug in $findRoute() that causes blow up on unmatched named route #10 (test closure scope). The `(function(relPath) {...})(rel)` form passes the value as an argument, sidestepping the `local` capture issue. Correct.
  • `##` escaping: confirmed correct in all string literals in the spec (`"issue #First 15 Minutes tutorial fails. The key [WO] does not exist. #2773"`, etc.).

Missed issues

  • `setting requestTimeout=30` in the fallback: the fallback copies the line from the surrounding context, but the original rationale was `application.wo.$getRequestTimeout() + 30` — a dynamic extension. In the fallback path Injector never loaded, so 30 s flat is reasonable; A doesn't flag this but it isn't a bug.
  • Content-Type header: the fallback writes HTML without an explicit `Content-Type: text/html` header. CFML pages default to `text/html` in all supported engines so this is benign in practice, but it could be paired with the HTTP 500 header when A's nit is addressed. Minor addition worth noting.
  • Spec location: the spec lives under `vendor/wheels/tests/specs/cli/` but validates files in `cli/lucli/templates/` and `public/`. Slightly unconventional (CLI-template regression in the framework suite), but not a defect — the `expandPath("/wheels/../..")` path resolution works correctly for this layout.

Verdict alignment

A's "comment" verdict with two non-blocking nits is consistent with the findings. The fix is correct and the nits are improvements, not blockers; "request-changes" would be too strong here.

Convergence

Aligned. A's findings check out; the docs commit that landed after A's review is clean; no false positives and no blocking missed issues. A's "comment" verdict with concrete actionable findings (HTTP 500 status code in both `Application.cfc` files; optional comment on the `catchClosePattern` assumption) is the right joint recommendation. Converging on changes so both improvements are applied before merge.

- Set HTTP 500 status code in the onError fallback in both
  cli/lucli/templates/app/public/Application.cfc and public/Application.cfc
  so monitoring tools and CDNs don't cache the Wheels-init failure as
  a successful response. Uses a plain struct for cfheader's
  attributeCollection per CLAUDE.md cross-engine invariant #10
  (Adobe CF 2023/2025 reject the arguments scope on built-in tags).
- Document the no-nested-braces assumption behind catchClosePattern in
  vendor/wheels/tests/specs/cli/OnErrorFallbackGuardSpec.cfc so a future
  edit that adds nested braces inside the outer catch knows why the
  silent fallback to scanFrom=1 is the safety net.
- Fix the contradictory recovery steps in the first-15-minutes guide
  (wheels reload requires a running server) at
  web/sites/guides/src/content/docs/v4-0-1-snapshot/start-here/first-15-minutes.mdx.
- Replace the speculative "pre-4.0.2" wording in
  .ai/wheels/troubleshooting/common-errors.md with "4.0.1 or earlier"
  since the fix is still in [Unreleased].

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

wheels-bot Bot commented May 21, 2026

Wheels Bot — Address Review (round 1)

Applied consensus findings from Reviewer A and Reviewer B's convergence (round 1):

  • HTTP 500 status code in the onError fallbackcli/lucli/templates/app/public/Application.cfc:281 and public/Application.cfc:295. Added a cfheader(attributeCollection=local.statusArgs) call (plain struct per CLAUDE.md cross-engine invariant Fixed bug in $findRoute() that causes blow up on unmatched named route #10) before the WriteOutput block so monitoring tools and CDNs see a real 5xx instead of caching a fake 200. Wrapped in its own try/catch so the body still renders if the header has already been written.
  • catchClosePattern assumption documentedvendor/wheels/tests/specs/cli/OnErrorFallbackGuardSpec.cfc:91. Added a comment above the regex explaining the no-nested-braces assumption and the scanFrom = 1 fallback, so a future edit that introduces a conditional or nested try inside the outer catch knows why the precision degrades silently.
  • First-15-minutes recovery steps fixedweb/sites/guides/src/content/docs/v4-0-1-snapshot/start-here/first-15-minutes.mdx. Replaced the contradictory "stop the server, run wheels reload, restart" with "while the server is running, try wheels reload; if that doesn't clear it, stop and restart." wheels reload is an HTTP request to the running server — it can't run against a stopped server.
  • Speculative version reference fixed.ai/wheels/troubleshooting/common-errors.md. Replaced "pre-4.0.2" with "4.0.1 or earlier" since the fix is still in [Unreleased] and the next release may not be named 4.0.2.

Tests re-run: hit the running local Lucee 7 server's /wheels/core/tests?db=sqlite&format=json endpoint after the edits. Full suite: 3689 pass / 0 fail / 0 error / 16 skipped (3705 specs across 226 bundles, 14.1s). wheels.tests.specs.cli.OnErrorFallbackGuardSpec: 2 pass / 0 fail / 0 error. (tools/test-local.sh itself has a sed -i '' BSD/GNU portability issue on this Linux runner, so I drove the in-process Lucee endpoint directly — same surface CI uses.)

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.

Copy link
Copy Markdown
Contributor Author

@wheels-bot wheels-bot Bot left a comment

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 round (e32399d0d) directly addresses all four outstanding nits from the two prior review passes — the missing HTTP 500 status code, the fragile catchClosePattern regex comment, the speculative version reference in the troubleshooting doc, and the contradictory wheels reload / stop-server recovery steps in the MDX guide. All fixes are correct and no new issues were introduced. Verdict: approve (submitted as comment — bot cannot self-approve).


Correctness

All four previously-flagged items are resolved:

  1. HTTP 500 status code — both cli/lucli/templates/app/public/Application.cfc and public/Application.cfc now emit a 500 before the fallback WriteOutput. The implementation follows the Adobe CF 2023/2025 attributeCollection pattern exactly (plain local struct, not arguments):

    local.statusArgs = {statusCode: 500, statusText: "Internal Server Error"};
    cfheader(attributeCollection=local.statusArgs);

    The surrounding try/catch handles the "header already written" edge case cleanly.

  2. Fragile catchClosePattern regexOnErrorFallbackGuardSpec.cfc now contains a comment block above the pattern explaining the [^\}]* assumption and the silent fallback to scanFrom = 1. The precision-loss scenario is documented; no logic change needed.

  3. Speculative version reference.ai/wheels/troubleshooting/common-errors.md now reads "4.0.1 or earlier" rather than the unreleased "pre-4.0.2" label. Correct.

  4. Contradictory MDX recovery stepsweb/sites/guides/src/content/docs/v4-0-1-snapshot/start-here/first-15-minutes.mdx now reads:

    While the server is running, try wheels reload to pick up the corrected mapping. If that does not clear it, stop and restart the server.

    The wheels reload call is now correctly placed inside the running-server context, matching the disjunction in the .ai/ reference doc.


Cross-engine

Colon-notation struct literals ({statusCode: 500, statusText: "Internal Server Error"}) are valid on Adobe CF 2018+ (the minimum supported version) and all Lucee/BoxLang targets. No new cross-engine concerns.


Tests

OnErrorFallbackGuardSpec.cfc is unchanged in logic; only the explanatory comment was added. The closure IIFE at line 35 ((function(relPath) {...})(rel)) still correctly captures the loop variable without triggering the inline-closure-as-constructor-arg crash (CLAUDE.md invariant #5 — this is an IIFE, not a named-arg). The private string function $stripCfmlComments is a test-component helper method, not a model/controller mixin, so private access is correct here.


Commits

fix(cli): address Reviewer A/B consensus findings (round 1)   — e32399d0d
docs(web/guides): note application error fallback and init failure in troubleshooting docs   — 71c35ec80
fix(cli): guard application.wo in onError so init failures don't cascade   — ad91b70e1

All three pass commitlint.config.js: valid types (fix, docs), subjects under 100 chars, sentence-case. DCO trailers are present on all commits.


Security

encodeForHTML(arguments.Exception.message) remains in place on both fallback render paths. No SQL, no secrets, no CSRF surface added.

@wheels-bot
Copy link
Copy Markdown
Contributor Author

wheels-bot Bot commented May 21, 2026

Wheels Bot — Reviewer B (round 1)

A's review of e32399d0d checks out. All four previously-flagged items are verifiably resolved in the diff, the cross-engine analysis is accurate, and the approve verdict is consistent with the findings. Converging on approve.

Sycophancy

None detected. A cites specific file + diff evidence for each of the four claims and notes exactly what changed. The "approve submitted as comment" framing is correct given the bot cannot self-approve.

False positives

None detected. Each of A's claims verified against the diff:

  • HTTP 500 in both Application.cfc files — confirmed. Both cli/lucli/templates/app/public/Application.cfc:281 and public/Application.cfc:295 now have the identical local.statusArgs = {statusCode: 500, statusText: "Internal Server Error"}; cfheader(attributeCollection=local.statusArgs); block inside a try/catch, before WriteOutput.
  • Colon-notation struct literal on Adobe CF 2018+ — A claims this is valid. Correct: {key: value} colon syntax is supported on Adobe CF 2018 (the framework's stated minimum) and all Lucee/BoxLang targets. No cross-engine concern.
  • catchClosePattern comment added — confirmed at OnErrorFallbackGuardSpec.cfc:89–106. The comment explains the [^\}]* no-nested-braces assumption and the scanFrom = 1 fallback.
  • Version reference fixed — confirmed: .ai/wheels/troubleshooting/common-errors.md now reads "4.0.1 or earlier", not "pre-4.0.2".
  • MDX recovery steps fixed — confirmed: first-15-minutes.mdx Aside reads "While the server is running, try wheels reload to pick up the corrected mapping. If that does not clear it, stop and restart the server." The logical ordering is correct.
  • private $stripCfmlComments in the spec — A correctly notes this is a test-component helper, not a model/controller mixin. CLAUDE.md cross-engine invariant New master - readme #7's public requirement applies to vendor/wheels/model/*.cfc and view helpers that go through $integrateComponents(). A spec CFC calling its own private method directly is fine on all engines.
  • IIFE loop capture — A's IIFE analysis is correct. (function(relPath) {...})(rel) passes the loop variable as a function argument; this is not the "inline closure as constructor named arg" crash (CLAUDE.md invariant New master - Microsoft Access Adapter #5), which specifically concerns new Foo(callback = function(){...}).

Missed issues

None blocking. One minor observation A didn't surface: the fallback writes HTML without an explicit Content-Type: text/html header. All supported CFML engines default to text/html for CFC requests, so this is benign in practice — not worth requesting changes over.

Verdict alignment

A's approve verdict is consistent with the findings: all four previously-consensus-agreed issues are resolved, no new issues introduced, cross-engine correctness confirmed.

Convergence

Aligned. All findings from the prior round are addressed. No false positives, no missed blocking issues, no sycophancy. Joint recommendation: approve.

Manually triggered baseline refresh via
.github/workflows/refresh-visual-baselines.yml
on branch fix/bot-2773-first-15-minutes-tutorial-fails-the-key-wo-does-no.

Run when an intentional content/layout change makes the visual-regression
check fail. The new PNG(s) under web/tests/visual-baselines/ are now the
expected rendering; re-run the failing visual-regression job to flip the
check green.
@bpamiri bpamiri marked this pull request as ready for review May 21, 2026 02:05
Copy link
Copy Markdown
Contributor Author

@wheels-bot wheels-bot Bot left a comment

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 round adds one commit — 73ddf07e8 (chore(web): refresh visual baseline(s) (all)) — authored by github-actions[bot] via the refresh-visual-baselines.yml workflow. It updates web/tests/visual-baselines/blog.png to reflect the <Aside> block added to first-15-minutes.mdx in round 2, unblocking the visual-regression CI check. All prior nits (HTTP 500 status code, catchClosePattern comment, speculative version reference, contradictory recovery steps) remain addressed. No new issues. Verdict: approve (submitted as comment — bot cannot self-approve).


Correctness

The only change is a binary PNG update to web/tests/visual-baselines/blog.png (121 KB → 130 KB). This is the expected mechanical consequence of the MDX <Aside> insertion in round 2: the guide page renders differently, so the visual-regression baseline must be refreshed or the CI check will stay red indefinitely. The refresh-baseline workflow is the correct mechanism for this.

No functional code was touched in this commit. All previously reviewed code — the StructKeyExists(application, "wo") guard in both Application.cfc files, the OnErrorFallbackGuardSpec.cfc spec, the .ai/ reference doc, and the MDX guide — is unchanged.


Commits

chore(web): refresh visual baseline(s) (all)   — 73ddf07e8

Type chore, scope web (valid per CLAUDE.md), subject 37 chars, sentence-case — passes commitlint.config.js.

The commit was authored by github-actions[bot] (not wheels-bot[bot]) and carries no Signed-off-by: trailer. This is a CI-generated commit from a first-party repo workflow — DCO App configurations typically allowlist github-actions[bot] and similar machine identities. Not escalating, but flag this if the DCO check redlines on this commit; the fix is to add the appropriate bot exemption in the DCO App settings or to add a Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> trailer to the workflow template.


Open nits from prior rounds (all resolved)

  1. HTTP 500 status code — resolved in round 3 (e32399d0d).
  2. Fragile catchClosePattern regex comment — resolved in round 3 (e32399d0d).
  3. Speculative version reference — resolved in round 3 (e32399d0d).
  4. Contradictory MDX recovery steps — resolved in round 3 (e32399d0d).

@wheels-bot
Copy link
Copy Markdown
Contributor Author

wheels-bot Bot commented May 21, 2026

Wheels Bot - Reviewer B (round 1)

A's review of 73ddf07e8 is accurate overall and the approve verdict is correct. One attributional inaccuracy in the reasoning, but it has no bearing on the verdict - the PNG refresh is appropriate regardless of which prior change triggered the visual-regression failure. Converging on approve.

Sycophancy

None detected. A cites the specific file, size delta, and causal chain rather than issuing a bare LGTM. The approve verdict for a PNG-only CI-unblocking commit is well-supported.

False positives

Minor - incorrect attribution of the blog.png change. A writes: "This is the expected mechanical consequence of the MDX Aside insertion in round 2." That inference is wrong. The visual-regression canary for the guides site points to /v4-0-0/start-here/tutorial/ (verified in web/scripts/visual-regression.mjs), not /v4-0-1-snapshot/start-here/first-15-minutes/ where the Aside lives. An Aside added to a v4-0-1-snapshot page cannot affect the guides.png canary, let alone the blog.png canary whose target is / (the blog index). The blog.png drift is more plausibly attributable to the Wheels 4.0.1 blog post added in d22f97c3d (#2772) appearing on the blog index - a baseline that was already stale before this PR started. The practical consequence is identical (baseline needed refreshing; all mode caught it), so the verdict is unaffected.

Missed issues

None blocking. One minor addition A could have made: the guides.png canary is pinned to /v4-0-0/start-here/tutorial/, so the Aside added to v4-0-1-snapshot/first-15-minutes.mdx is invisible to the visual regression suite entirely. Not a defect in this PR, but worth noting if visual coverage of v4-0-1-snapshot pages is ever desired.

Verdict alignment

A's approve verdict is consistent with the findings: the only code change in this commit is a mechanical PNG baseline refresh, all four previously-consensus-agreed fixes remain in place, and the commit message passes commitlint. Correct call.

Convergence

Aligned. The misattribution of the blog.png cause is inconsequential - the refresh action is correct regardless. No false positives that change the verdict, no missed blocking issues, no sycophancy. The DCO concern A raised is handled correctly: prior github-actions[bot] baseline-refresh commits in this repo (e.g. cfae9d1e) follow the same no-sign-off pattern and have landed without DCO failures, indicating the App already exempts machine identities. Joint recommendation: approve.

@bpamiri bpamiri merged commit 1f4d729 into develop May 21, 2026
2 checks passed
@bpamiri bpamiri deleted the fix/bot-2773-first-15-minutes-tutorial-fails-the-key-wo-does-no branch May 21, 2026 02:40
bpamiri added a commit that referenced this pull request May 21, 2026
…sting

Resolves CHANGELOG.md conflict in the [Unreleased] / Fixed section:
both this branch and #2774 (just merged to develop) added a new
[Unreleased] block at the same anchor. Kept develop's H1 header form
(`# [Unreleased]`, matching the 4.0.1 release header style) and merged
both fix bullets under a single ### Fixed list — the .deb/.rpm framework
nesting fix from this branch first (root cause), then #2774's defensive
onError guard second (defensive belt for the same #2773 cascade).

Both fixes close #2773 from different angles, so listing them together
is accurate.

Signed-off-by: Peter Amiri <peter@alurium.com>
bpamiri added a commit that referenced this pull request May 21, 2026
…be_* (#2777)

* docs(web/guides): correct Linux bleeding-edge install URLs to wheels-be_*

PR #2759 (2026-05-18) renamed the snapshot Linux artifacts from `wheels_*`
to `wheels-be_*` (debs) and `wheels-be-*.x86_64.rpm` (rpms) so the package
name itself differentiates the channel. The install guides were not
updated alongside that rename, so every documented `curl -fsSLO ...`
command for Linux bleeding-edge install resolves to a 404 against the
actual snapshot release assets.

Verified against v4.0.2-snapshot.1923 (published 2026-05-20):

  Guide says:    .../wheels_4.0.2.snapshot.1923_amd64.deb   → 404
  Actual asset:  .../wheels-be_4.0.2.snapshot.1923_amd64.deb

Fix all six pages where the snippets / prose examples appear (three
unique pages mirrored across v4-0-0 and v4-0-1-snapshot doc versions):

  start-here/installing.mdx                 — "Want bleeding-edge?" aside
  start-here/release-channels.mdx           — main BE install snippets +
                                              "Switching channels" snippets
                                              + tilde-mangling prose
  command-line-tools/installation.mdx       — bleeding-edge install snippets

The substitutions are scoped to bleeding-edge contexts (snippets using
`${SNAP_FILENAME_VER}` and prose `wheels_4.0.0.snapshot.*` filename
examples). Stable-channel snippets, which use `${WHEELS_VERSION}` and
fetch from `wheels-dev/wheels` (not `wheels-snapshots`), are unchanged —
they correctly retain the bare `wheels_` / `wheels-` prefixes because
the stable package name on Linux is still just `wheels`.

Without this fix, users cannot install or test bleeding-edge / develop
snapshots on Linux via the documented flow. This blocks user-side
verification of develop-only fixes before they ship in the next stable
patch — including PR #2776 (Linux .deb framework nesting fix) and
PR #2774 (defensive onError guard), both of which close issue #2773.

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

* docs(web/guides): fix release-channels.mdx — missed BE Tab URLs + Linux switching semantics

Round-1 reviewer findings on PR #2777:

A's Nit 1 — primary install Tabs at lines 104-105 (Debian/Ubuntu BE)
and 112 (Fedora/RHEL BE) of `release-channels.mdx` still resolved to
404. My initial verification sweep grep'd for `${SNAP_FILENAME_VER}`,
but these snippets bind the tag to `${WHEELS_FILENAME_VER}` (a
different bash var name). The fix is the same — point at the
`wheels-be_` / `wheels-be-` artifacts.

A's Nit 2 + B's catch — the "Switching channels" section had three
related staleness bugs after #2759 renamed the BE package:

  1. Line 129 prose claimed "only a single package name (`wheels`)
     is published per channel today" — false post-rename.

  2. Lines 142-143 inline comment ("upgrades in place — no uninstall
     step needed") was true when both channels shared the `wheels`
     name, but the new world depends on the actual nfpm-declared
     `Replaces:` / `Conflicts:` metadata. B caught the contradiction
     between A's proposed line-129 prose and the existing line-142
     comment.

  3. Lines 158-172 (Linux BE → stable, both Debian and Fedora) had
     the *same* conceptual bug as 142-143: they prescribed
     `--allow-downgrades` (apt) / `dnf downgrade`, both of which
     assume same-package-name version transitions. With different
     names, both would fail with a `/usr/bin/wheels` file conflict
     because the stable `wheels` package doesn't declare
     `Replaces:`/`Obsoletes: wheels-be`. Reviewers didn't explicitly
     flag this set, but it's the same root cause and listing them
     inconsistently would have left readers worse off.

Verified the actual nfpm metadata before rewriting (so the prose
matches what the packages really declare):

  wheels-be deb:  Replaces: wheels  +  Conflicts: wheels
  wheels-be rpm:  Conflicts: wheels  (no Obsoletes)
  wheels    deb:  no Replaces/Conflicts against wheels-be
  wheels    rpm:  no Conflicts/Obsoletes against wheels-be

The new prose at line 129 explains the asymmetry up front; each
snippet now carries a short comment naming the specific metadata
that drives its action (or the lack of metadata that requires the
explicit `apt remove` / `dnf remove`).

Stable-channel snippets and stable install Tabs are unchanged.

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

* docs(web/guides): name the actual Conflicts declaration in BE→stable comments

Reviewer A round-2 nit on PR #2777: the BE → stable (Debian) snippet's
comment said apt "would fail with a /usr/bin/wheels file conflict",
framing the failure mode as a dpkg-level file-ownership conflict. The
actual blocker is the package-level `Conflicts: wheels` declaration in
wheels-be's deb metadata — apt refuses the install with a package
conflict error before dpkg ever attempts to unpack files. An advanced
user debugging the actual error message would be confused by the
file-conflict framing.

Rewrite the Debian comment per A's suggestion, naming the actual
mechanism: `wheels-be declares Conflicts: wheels`. Kept the secondary
note about the missing `Replaces: wheels-be` in stable since it
explains why apt also wouldn't auto-remove (relevant context if a
reader wonders whether a single command could swap them).

Updated the Fedora BE → stable comment to use parallel framing for
consistency — same root cause (`wheels-be` declares `Conflicts:
wheels`, applies bidirectionally on rpm too). Reviewer A only flagged
the Debian site explicitly, but leaving the two comments inconsistent
would have invited the same "two sites must agree" finding that
caught round 1's line-142 / line-129 contradiction.

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

---------

Signed-off-by: Peter Amiri <peter@alurium.com>
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.

First 15 Minutes tutorial fails. The key [WO] does not exist.

1 participant