fix(seed): fail and roll back when seedOnce entries fail validation

[bpamiri]

Summary

Seeder.runSeeds() returned success=true ("Seeding complete") and committed the wrapping transaction even when seedOnce() recorded action='failed' results — a half-applied seed run was indistinguishable from a fully-applied one in CI and wheels seed output.

Changes:

• seedOnce() now increments a new totalFailed counter on the failed branch.
• runSeeds() rolls back the entire run when any entry failed and returns success=false with the failed entries named (model: first error message, via a new $failedEntriesSummary() helper). totalFailed is included in every return shape.
• Documented design choice (the issue asked for an explicit decision): rollback over commit-with-report. The docblock already promised atomicity, seedOnce() is idempotent so a corrected rerun re-applies everything, and commit-with-report reproduces exactly the half-applied ambiguity this issue complains about.
• runDbSeed() in the dev-UI bridge copies totalFailed into the response envelope. Since parseCliResponse already throws Wheels.Cli.CommandFailed on success:false, wheels seed now exits non-zero when any entry fails — no CLI change needed.

Fixes #2973

Type of Change

• Bug fix

Test Plan

• New fixture tests/_assets/seeder/partialfailure/seeds.cfm (one valid User entry + one missing required properties)
• New specs: "returns failure and rolls back when a seedOnce entry fails validation" (asserts success=false, totalFailed=1, failed model named in message, and the successful sibling entry rolled back) and "counts failed entries and reports them in the result" — both red before, green after
• Bundle: 17 pass / 0 fail / 0 error
• Full core suite (Lucee 7 + SQLite): 4269 pass / 0 fail / 0 error
• CHANGELOG entry under [Unreleased]

🤖 Generated with Claude Code

[wheels-bot]

Wheels Bot — Reviewer A

TL;DR: This PR correctly fixes the silent-success bug in Seeder.runSeeds() when validation failures occur — the logic is sound, the atomicity test is solid, and the new totalFailed counter flows correctly through every return shape. Two minor code nits and one doc gap are worth addressing before merge, but nothing here rises to a request-for-changes level. Verdict: comment.

---

Correctness

The fix is logically correct. All four return paths in runSeeds() now include totalFailed:

Path	totalFailed in return?
No seed files found (early return)	✓ hardcoded 0
Validation failure branch	✓ this.totalFailed
Thrown exception	✓ this.totalFailed
Success	✓ hardcoded 0

The rollback happens before the early return on the failure branch, so the transaction is always properly resolved before the function exits.

One minor inconsistency: in the success return (Seeder.cfc line 128), totalFailed is hard-coded to 0 while the sibling counters use the live this values:

// vendor/wheels/Seeder.cfc, line 121–129
return {
    ...
    totalCreated = this.totalCreated,
    totalSkipped = this.totalSkipped,
    totalFailed = 0              // hard-coded; siblings use this.*
};

The value is correct right now (the failure branch returns early, so we only reach this line when this.totalFailed == 0), but if the branch logic ever changes the inconsistency becomes a latent bug. Using this.totalFailed costs nothing and removes the discrepancy.

---

Conventions

Minor — redundant guard in cli.cfm (line 893):

// vendor/wheels/public/views/cli.cfm, line 893
if (structKeyExists(conventionResult, "totalFailed")) {
    result.totalFailed = conventionResult.totalFailed;
}

All four runSeeds() return paths now include totalFailed, so this guard is always true. The defensive style is harmless but implies to the next reader that the key might be absent — which is no longer the case. A simple unconditional assignment would be clearer.

---

Tests

Tests look good. BDD syntax, extends wheels.WheelsTest, covers both the counter-increment in seedOnce() and the full atomicity guarantee (the rollback test verifies the successfully-saved first entry is absent after the run). The cleanup-before-run pattern is the right call for a test that must start from a clean state.

One thing to note: the counts failed entries unit test (line 176 of seederSpec.cfc) resets seeder.totalFailed = 0 but leaves the shared instance's results array intact. runSeeds() resets this.results on each call, but this unit test calls seedOnce() directly on the shared instance. $failedEntriesSummary() iterates this.results, not this.totalFailed, so stale failed entries from a prior spec could surface in a summary. There is no active failure here (the spec does not call $failedEntriesSummary() directly), but worth keeping in mind if the test is extended.

---

Docs

seeding.mdx needs two small updates to match the new behavior.

Line 53 currently says:

"If any call throws, the whole file rolls back"

Line 103 says:

"if any of them throws, nothing commits"

Both describe only the exception-based rollback path. This PR adds a second rollback trigger: a seedOnce() entry that fails model validation (save() returns false — no exception thrown). A user reading the guide today would not know that a silently failing validatesPresenceOf check also causes a full rollback and a non-zero exit code.

Suggested replacement for line 53:

"If any call throws or fails model validation, the whole file rolls back — you never end up half-seeded."

Suggested replacement for line 103:

"if any of them throws or fails validation, nothing commits"

A sentence noting that wheels seed now exits non-zero on validation failures would also be useful (the mechanism — success: false → Wheels.Cli.CommandFailed — is already there; it just isn't documented).

---

Commits

Both commits are valid conventional-commit format:

• fix(seed): fail and roll back when seedOnce entries fail validation
• chore(docs): move changelog entry to changelog.d fragment

Types and scopes are within the allowed set; headers are within 100 chars.

---

Security

No concerns. Validation error messages in $failedEntriesSummary() come from model config (developer-controlled), not user input.

[wheels-bot]

Wheels Bot - Reviewer B (round 1)

A's review is accurate and well-calibrated. The core fix is correctly verified, all four return-path checks against the diff are right, and the three findings A raised are genuine. No sycophancy, no false positives, no meaningful missed issues. I agree on convergence with changes.

Sycophancy

None detected. A gives specific findings rather than blanket praise, and correctly declined to approve without reservation on a transactional/seeding-behavior change.

False positives

None detected. I verified each claim against the diff:

• Hardcoded totalFailed = 0 on the success path (Seeder.cfc ~line 128): confirmed -- totalCreated and totalSkipped use this.* on the same return, totalFailed is a literal. A correctly notes the value is safe right now (the failure branch exits early, so this line is only reached when this.totalFailed == 0), but the inconsistency is real.
• Redundant structKeyExists guard in cli.cfm line 893: confirmed -- all four runSeeds() return paths in this diff include totalFailed (no-files path: hardcoded 0; failure path: this.totalFailed; exception path: this.totalFailed; success path: hardcoded 0). The guard is always true.
• seeding.mdx doc gap: A's reasoning is sound. The current guide describes only exception-based rollback; validation failures are now a second rollback trigger and are not mentioned. The changelog fragment does not substitute for the user-facing guide.

Missed issues

None that would change the verdict.

One minor observation A could have surfaced but did not: the unit test at line 176 resets seeder.totalFailed = 0 but leaves seeder.results with any entries from prior specs on the shared instance. The spec only asserts totalFailed and local.result.action, so this does not cause a failure -- and A flags this themselves in the Tests section, so it is addressed.

Verdict alignment

A's "comment" verdict is consistent with the findings -- two code-style nits and a docs gap, none of which block the fix from being correct and safe.

Convergence

Aligned. A's three findings are valid and actionable:

1. Seeder.cfc success-path return: change totalFailed = 0 to totalFailed = this.totalFailed.
2. cli.cfm line 893: make result.totalFailed = conventionResult.totalFailed unconditional (drop the structKeyExists guard).
3. web/sites/guides/src/content/docs/v4-0-0/basics/seeding.mdx lines 53 and 103: add validation-failure as a rollback trigger alongside thrown exceptions; add a sentence that wheels seed now exits non-zero when any entry fails.

Recommending apply.