fix(xim): surface multi-repo install failures + best-effort catalog (#374) [0.4.67]#375
Merged
Merged
Conversation
…374) `interface install_packages`/`plan_install` exited 1 with an EMPTY NDJSON stream whenever a project configured >=2 index_repos and one was degenerate (no pkgs/, unreachable, empty default-namespace redirect target). Consumers (mcpp) saw `{"exitCode":1,"kind":"result"}` with zero diagnostics. Two independent architectural defects combined: 1. Observability: the xim command layer reported failures via the global log::error(), which interface mode (platform::set_tui_mode(true)) fully suppresses. The NDJSON protocol has a first-class ErrorEvent kind that subos.cppm already uses — cmd_install just never did. 2. Multi-repo robustness: PackageCatalog::rebuild() and sync_all_repos() fail-fast on the first bad repo, so one degenerate index_repo collapses the whole catalog — even the healthy repos. (Sub-index repos were already best-effort right next to the fatal project-repo path.) Fix (design doc §4): * P0 — cmd_install emits structured ErrorEvent (code + hint + searched repos) at every resolve/build/plan/install failure point, instead of a swallowed log::error. interface::run adds a backstop invariant: a non-zero exit with no ErrorEvent synthesizes one, so the stream is never empty on failure. * P1 — catalog.rebuild() and sync_all_repos() are best-effort: a repo that can't load/sync is skipped and recorded, the rest keep working, and the skip is surfaced on the wire as a warn LogEvent. rebuild hard-fails only when NOTHING loaded. Matches the sub-index precedent. Adds e2e E2E-29 (interface_multi_repo_error_visibility_test.sh): A1 healthy target resolves despite a broken sibling repo (exit 0 + skip surfaced); A2 missing target -> non-zero + {"kind":"error"} on the wire. Verified: RED on 0.4.66, GREEN on 0.4.67; 11/11 unit tests pass. Bumps version 0.4.66 -> 0.4.67. Refs: .agents/docs/2026-07-19-issue374-multi-repo-silent-exit-design.md
Code-review follow-up on the non-zero-exit backstop added in the previous
commit. Two paths returned non-zero without routing through the session, so
the generic backstop fired with a misleading E_INTERNAL:
* CancelledException (client `{"action":"cancel"}`, exit 130) emitted no
ErrorEvent → backstop synthesized `E_INTERNAL "... failed with no error
detail"` for a NORMAL, expected cancellation. Now emits the dedicated
E_CANCELLED (recoverable) — which also finally makes that enum value
live — so saw_error() is true with the correct code and the backstop
is bypassed.
* std::exception wrote its error straight to std::cout (not via session),
leaving saw_error() false → backstop emitted a SECOND, generic error on
top of the specific one. Now routed through session.emit_event so it is
counted and not duplicated.
Strengthens test_interface_protocol's plan_install-missing case to assert
the #374 guarantee directly: non-zero exit + a structured E_NOT_FOUND error
on the wire (was only checking that some NDJSON was produced).
Verified: mcpp test 11/11; e2e interface_multi_repo_error_visibility GREEN.
…ation) The E2E-29 test pointed the global index at tests/fixtures/xim-pkgindex and wrote an xim-indexrepos.lua into it to neutralise sub-index discovery — mutating a shared, checked-in fixture (leaves an untracked file; unsafe if other tests read it). Build a dedicated local global index under the runtime dir instead, so the test is fully self-contained and offline and never touches tests/fixtures. Verified: A1/A2/A2b GREEN, fixture tree stays clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #374.
Symptom
xlings interface install_packages/plan_installexited 1 with an empty NDJSON stream whenever a project configured ≥2index_reposand one of them was degenerate (nopkgs/, unreachable URL, empty default-namespace redirect target). Consumers (mcpp, mcpp-community/mcpp#238) saw only:{"exitCode":1,"kind":"result"}…with zero diagnostics. Single-repo configs worked. Reproduced with a clean single-vs-double comparison (see design doc §3).
Two root causes (architectural)
log::error(), which interface mode (platform::set_tui_mode(true)) fully suppresses (not even stderr). The NDJSON protocol already has a first-classErrorEventkind thatsubos.cppmuses —cmd_installjust never did. This affected every exit-1 path, not only multi-repo.PackageCatalog::rebuild()andsync_all_repos()fail-fast on the first bad repo, so one degenerateindex_repocollapsed the whole catalog (healthy repos included). Sub-index repos were already best-effort right next to the fatal project-repo path.Fix
cmd_installemits a structuredErrorEvent(code + hint + searched-repos) at every resolve/build/plan/install failure point instead of a swallowedlog::error.interface::runadds a backstop invariant: a non-zero exit that emitted noErrorEventsynthesizes one, so the stream is never empty on failure.catalog.rebuild()andsync_all_repos()are best-effort — a repo that can't load/sync is skipped and recorded, the rest keep working, and the skip is surfaced on the wire as awarnLogEvent.rebuildhard-fails only when nothing loaded. Mirrors the existing sub-index precedent.Result for the reported scenario: healthy repos still resolve packages (exit 0) and the skipped repo is visible on the wire; genuinely-missing targets exit non-zero with a structured
E_NOT_FOUNDerror naming the searched repos.Verification
interface_multi_repo_error_visibility_test.sh:{"kind":"error"}on the wire (bothplan_installandinstall_packages).{"exitCode":1}), GREEN on 0.4.67.mcpp test: 11/11 unit binaries pass (incl.test_interface_protocol).Bumps version 0.4.66 → 0.4.67.
Design doc:
.agents/docs/2026-07-19-issue374-multi-repo-silent-exit-design.md