fix(frontseat-plugin-backstage): GH-128 surface catalog decode errors

[jbadeau]

Fixes #128: a catalog document that failed to decode (the issue's example: tools: [node] where a mapping is expected) was silently skipped mid-file, and a whole-file load failure was demoted to a stderr warning inside the plugin process — either way the entity silently vanished from the catalog.

Both paths now return an error naming the file and the document index, so discovery fails loud instead of building against a partial catalog. Regression test included.

🤖 Generated with Claude Code

[joaodinissf]

This makes it worse, not louder. The host swallows the error — the plugin.Entities(...) branch in Manager.DiscoverEntities (libs/frontseat-plugin-host/discover.go):

result, err := plugin.Entities(ctx, ws, matchedFiles)
if err != nil {
    logger.Warn("plugin failed to create entities", ...)
    continue
}

Warn + continue. So one bad file means backstage returns nil and contributes zero entities, discovery still reports success, and the empty map is cached. One bad document costs the whole catalog instead of one entity.

Also folds a transient read error on file 3 into total catalog loss.

Fix: keep the per-file continue here and return a partial result alongside the error, or make the host treat an Entities error as fatal. The plugin alone cannot make this loud.

[jbadeau]

Fixed in 994fe9f. The host no longer swallows the error: the plugin.Entities branch in Manager.DiscoverEntities now returns fmt.Errorf("plugin %s failed to create entities in %s: %w", ...) instead of warn-and-continue, so a genuinely broken catalog fails discovery loudly rather than caching an empty map as success. That's the half the plugin couldn't fix on its own — so Fixes #128 is now accurate.

Paired with the plugin change so a transient read error / syntax error is fatal (loud), while a wrong-shape document is reported-and-skipped without taking its siblings down (see the other thread).

---

Generated by Claude Code

[joaodinissf]

errors.Is(err, io.EOF) replacing the err.Error() == "EOF" string compare is a real fix on its own — good. So is terminating the loop: on a YAML syntax error the previous code spun forever, because Decode returns the identical error on every call and never reaches EOF.

But aborting the whole file on any decode error also drops the valid documents beside the bad one, which is the opposite of what #128 asks for. #128 wants the bad entity to stop vanishing silently; it does not ask for its siblings to vanish too.

Suggest: warn naming file and document index, keep the successfully decoded entities, and continue — but only for the error class where continuing is safe. The discriminator has to be the error type: continue on a *yaml.TypeError, where the decoder advances to the next document, and stop on anything else. "Did the entity come out populated?" cannot discriminate, because a syntax error also yields an empty entity.

[jbadeau]

Done in 994fe9f — discriminating on error type exactly as you laid out:

var typeErr *yaml.TypeError
if errors.As(derr, &typeErr) {
    skipped = append(skipped, fmt.Sprintf("backstage: catalog %s document %d has an invalid shape, skipping: %v", path, doc, derr))
    continue
}
return entities, skipped, fmt.Errorf("document %d: %w", doc, derr)

continue on *yaml.TypeError (the decoder has advanced, siblings survive), abort on anything else (syntax error, which never advances or reaches EOF). Kept errors.Is(err, io.EOF). The skip is surfaced via plugin stderr (backstage:-prefixed), so the bad document stops vanishing silently without dragging its siblings with it. TestDecodeCatalogDocuments_TypeErrorKeepsSiblings pins the bad-doc-plus-valid-sibling case; TestLoadEntitiesFromFiles_SyntaxErrorSurfaces pins the abort path.

Also extended to both catalog rules — catalog-schema and catalog-best-practices now emit a violation on an unparseable catalog-info.yaml instead of dropping the error, so it's visible to conformance as well as discovery.

---

Generated by Claude Code

[joaodinissf]

Requesting changes — this is a regression on the real path.

Two inline. The core one: the host warn-and-continues on an Entities error (the plugin.Entities branch in Manager.DiscoverEntities), so returning early means backstage contributes zero entities while discovery still reports success. One bad document used to cost one entity; now it costs the whole catalog.

Worth keeping: errors.Is(err, io.EOF) is right, and aborting genuinely terminates a real hang — the previous loop spun forever on a YAML syntax error, since Decode returns the same error on every call and never reaches EOF. The fix needs to preserve that while not discarding valid documents.

The same silent continue survives in both catalog rules — each calls loadEntityFromFile and drops the error on the floor (rule_catalog_schema.go, rule_catalog_best_practices.go), so an unparseable catalog file is invisible to conformance as well as to discovery. Whatever this PR settles for an Entities error should apply at all three sites.

Fixes #128 is premature until the host stops swallowing the error.

This review was published with assistance from Claude.