Skip to main content
← Back to list
01Issue
BugShippedSwamp CLI
Assigneesstack72

#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 ValidationFailed as a queryable signal; in practice, schema-invalid content produces BundleBuildFailed and 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 lastError message string to distinguish, instead of pattern-matching on stateTag.
  • Test-matrix simplification blocked. The swamp-uat suite currently asserts BundleBuildFailed for schema-invalid content (per the canonical pattern from #334) because that's what production produces. Once this issue ships, the assertion changes to ValidationFailed and 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:

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

  1. The reproduction below produces stateTag: "ValidationFailed" (not BundleBuildFailed):
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"
  1. An equivalent reproduction with unresolvable-import content (bundle throw) STILL produces BundleBuildFailed (no regression on #334).
  2. recordValidationFailed retains the existing bundle and updates the fingerprint, per invariant I3 (row_state.ts:32). Same pattern as the BundleBuildFailed fingerprint-update from #334.
  3. No regression in doctor_extensions_failure_states_test.ts (from #334) — the unresolvable-import test must still pass with BundleBuildFailed.
  4. Recovery path: ValidationFailed → fix schema → next reconcile → Indexed. Same stability pattern as BundleBuildFailed recovery.

Files an implementing agent should read first

  • src/domain/extensions/extension_loader.ts:389-459 — the two throw sites
  • src/libswamp/extensions/reconcile_from_disk_service.ts:483-507 — the catch block to bifurcate
  • src/domain/extensions/extension.ts:343-365recordValidationFailed signature (needs same fingerprint+sourceMtime treatment that #334 added to recordBundleBuildFailed)
  • src/domain/extensions/row_state.ts:31-33 — the I3 invariant documentation for ValidationFailed
  • src/domain/extensions/bundle_freshness.ts:398markCatalogValidationFailed for 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 on registries.failures[] per the dual-path workaround. After this fix, can assert directly on sourceDetails[].stateTag === "ValidationFailed".
  • Some assertions in state/bundled_test.ts and state/indexed_test.ts may need adjustment.

Same coordination pattern as #334: the implementer's PR includes the swamp-uat assertion updates as a follow-up commit/PR.

This issue does NOT:

  • Remove markCatalogValidationFailed from bundle_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 BundleBuildFailed is recorded (that was #334's scope).

Keep the diff bounded to: new ValidationError type + reconcile catch bifurcation + recordValidationFailed signature update + fingerprint/recovery tests.

  • #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).
02Bog Flow
OPENTRIAGEDIN PROGRESSSHIPPED+ 1 MOREASSIGNED+ 5 MOREREVIEW+ 3 MOREPR_MERGEDCOMPLETE

Shipped

5/13/2026, 1:35:11 PM

Click a lifecycle step above to view its details.

03Sludge Pulse
stack72 assigned stack725/13/2026, 12:42:48 AM

Sign in to post a ripple.