#340 Distinguish validation-throw from bundle-throw in reconcile catch; surface ValidationFailed in sourceDetails
Opened by stack72 · 5/12/2026· Shipped 5/13/2026
Summary
ValidationFailed is a documented RowState (see row_state.ts:31-33) with a
domain-layer mutator (recordValidationFailed in extension.ts:343), but
zero production callers. The reconcile catch block at
reconcile_from_disk_service.ts:483-507 collapses both bundle-build throws
AND Zod-validation throws into a single recordBundleBuildFailed write.
Result: ValidationFailed is unreachable in aggregateState.sourceDetails[]
through any current CLI path, even though the contract documents it.
The legacy path's markCatalogValidationFailed (bundle_freshness.ts:398)
DOES write ValidationFailed rows, but it bypasses the Extension aggregate
(direct catalog upsert), so I-Repo-1 doesn't fire on that path. After #334
ships, the legacy path is increasingly de-prioritized as the reconcile path
becomes the canonical failure-recording mechanism — making the
ValidationFailed gap more visible.
Why this matters
- Documented contract not honored. The 7-RowStates surface promises
ValidationFailedas a queryable signal; in practice, schema-invalid content producesBundleBuildFailedand the user can't tell the difference between "bundle didn't build" and "bundle built but schema rejected the export." - Diagnostic value lost. The two failure classes have different root
causes and different fix paths. Collapsing them means users debugging
extension failures have to read the
lastErrormessage string to distinguish, instead of pattern-matching onstateTag. - Test-matrix simplification blocked. The swamp-uat suite currently
asserts
BundleBuildFailedfor schema-invalid content (per the canonical pattern from #334) because that's what production produces. Once this issue ships, the assertion changes toValidationFailedand tests become more semantically accurate.
Root cause
extension_loader.ts:bundleAndIndexOne (lines 389-459) throws in two
distinct places:
// Bundle-build throw — bundler/transpiler failure (unresolvable import,
// syntax error, top-level throw during module evaluation).
const { js, fromCache } = await this.bundleWithCache(...); // ← can throw
const module = await this.importBundle(...); // ← can throw
// Validation throw — bundle built cleanly, schema rejected export.
const parsed = this.adapter.validatePrimaryExport(...);
if (!parsed.success) {
throw new Error(this.adapter.formatValidationError(parsed.error)); // ← line 423
}Both throws are caught by reconcile's single catch block at line 483, which
unconditionally calls recordBundleBuildFailed. The throw site information
is lost.
Proposed fix
Introduce a discriminated error type so the catch can route correctly. Three sub-options:
Option A — Custom error subclass (recommended)
Define ValidationError extending Error in
src/domain/extensions/errors.ts. Throw it at extension_loader.ts:423
and :446. In reconcile's catch:
} catch (error) {
const errorMsg = error instanceof Error ? error.message : String(error);
const fp = await tryComputeFingerprint(...); // from #334's helper
if (error instanceof ValidationError) {
ext = recordValidationFailed(ext, {
location: effectiveLoc,
lastError: errorMsg,
fingerprint: fp,
// ValidationFailed retains the existing bundle per I3 — read it
// from the existingSource if available.
bundle: existingSource?.state.tag === "Bundled" ? ... : undefined,
});
} else {
ext = recordBundleBuildFailed(ext, { ... }); // existing path
}
}Pros: type-safe, clear at throw and catch sites. Cons: small new module.
Option B — Tagged return type from bundleAndIndexOne
Change the return to { ok: true; ... } | { ok: false; reason: "bundle" | "validation"; error } instead of throwing. Catch becomes a switch on reason.
Pros: no exception-based control flow.
Cons: bigger refactor; touches every caller of bundleAndIndexOne
(install service, reconcile service).
Option C — Sniff the error message in the catch
Match on lastError string content to decide which mutator to call.
Pros: smallest diff. Cons: brittle, breaks on i18n / error-message refactors. Don't do this.
Recommendation: Option A. Smallest reasonable diff, type-safe, doesn't churn callers.
Acceptance criteria
- The reproduction below produces
stateTag: "ValidationFailed"(notBundleBuildFailed):
swamp repo init
mkdir -p extensions/models/test
cat > extensions/models/test/entry.ts <<'TS'
export const model = "not an object";
TS
swamp doctor extensions --json --verbose
# → sourceDetails[].stateTag === "ValidationFailed"- An equivalent reproduction with unresolvable-import content (bundle
throw) STILL produces
BundleBuildFailed(no regression on #334). recordValidationFailedretains the existing bundle and updates the fingerprint, per invariant I3 (row_state.ts:32). Same pattern as theBundleBuildFailedfingerprint-update from #334.- No regression in
doctor_extensions_failure_states_test.ts(from #334) — the unresolvable-import test must still pass withBundleBuildFailed. - Recovery path:
ValidationFailed→ fix schema → next reconcile →Indexed. Same stability pattern asBundleBuildFailedrecovery.
Files an implementing agent should read first
src/domain/extensions/extension_loader.ts:389-459— the two throw sitessrc/libswamp/extensions/reconcile_from_disk_service.ts:483-507— the catch block to bifurcatesrc/domain/extensions/extension.ts:343-365—recordValidationFailedsignature (needs same fingerprint+sourceMtime treatment that #334 added torecordBundleBuildFailed)src/domain/extensions/row_state.ts:31-33— the I3 invariant documentation forValidationFailedsrc/domain/extensions/bundle_freshness.ts:398—markCatalogValidationFailedfor reference (the existing legacy path that writes ValidationFailed via direct upsert; this issue does NOT remove it, see related context)
UAT coupling
When this lands, the swamp-uat tests that assert BundleBuildFailed for
schema-invalid content need to flip to ValidationFailed. Specifically:
tests/cli/extension/state/validation_failed_test.ts— currently asserts onregistries.failures[]per the dual-path workaround. After this fix, can assert directly onsourceDetails[].stateTag === "ValidationFailed".- Some assertions in
state/bundled_test.tsandstate/indexed_test.tsmay need adjustment.
Same coordination pattern as #334: the implementer's PR includes the swamp-uat assertion updates as a follow-up commit/PR.
Related context — out of scope
This issue does NOT:
- Remove
markCatalogValidationFailedfrombundle_freshness.ts. That is the legacy path; removing it is part of the W7 surface-unification work (separate issue). - Eliminate the
registries.<kind>.failures[]surface. Same — W7. - Change how
BundleBuildFailedis recorded (that was #334's scope).
Keep the diff bounded to: new ValidationError type + reconcile catch
bifurcation + recordValidationFailed signature update +
fingerprint/recovery tests.
Related issues
- #334 — sequencing fix that made
sourceDetails[]always-populated (prerequisite, not a blocker) - swamp-club#209 — the original "rebundle loop on stable broken source"
finding that motivated
markCatalogValidationFailed. Same invariant applies here (fingerprint must update so the loop doesn't reignite).
Shipped
Click a lifecycle step above to view its details.
Sign in to post a ripple.