From d9fa4d661370975da820f9833244bc8577c5fa25 Mon Sep 17 00:00:00 2001 From: kami Date: Mon, 20 Jul 2026 01:08:11 +0400 Subject: [PATCH] Fail closed on Nexus/Hexis dependency errors, accept flat resolve shape MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Vikunja #268 (P0): handleHexisAct swallowed genuine Nexus resolve errors and Hexis capability-discovery errors into "" or an empty capability list, which fell through to the local system command executor — a dependency outage silently looked identical to "not an ecosystem entity" or "no capabilities registered", violating the spec's degrade-independently / never-silent-all-clear invariant. - resolveEntityReference's error is now distinguished from a legitimate not_found: only the latter falls through. - discoverCapabilities now returns (caps, err) instead of collapsing a Hexis failure into an empty slice; a real error stops the action with a degraded-mode spoken reply instead of reaching h.tools.Exec. - nexusResolveResult gains a custom UnmarshalJSON to accept the flat entity_id/entity_type/display_name shape from ECOSYSTEM-SPEC.md §1.5 (Nexus now emits both shapes; Maven now reads both). - Added regression tests: flat-shape resolve, Nexus error fails closed, Hexis error fails closed, not_found still falls through to local exec. --- cmd/mavend/ecosystem.go | 35 +++++++++++-- cmd/mavend/ecosystem_test.go | 98 ++++++++++++++++++++++++++++++++++++ cmd/mavend/voice.go | 17 +++++-- 3 files changed, 141 insertions(+), 9 deletions(-) diff --git a/cmd/mavend/ecosystem.go b/cmd/mavend/ecosystem.go index 3e5ae07..3f51f9f 100644 --- a/cmd/mavend/ecosystem.go +++ b/cmd/mavend/ecosystem.go @@ -12,8 +12,8 @@ import ( "net/http" "time" - "github.com/kami/maven/internal/config" hexisclient "github.com/kami/hexis/pkg/client" + "github.com/kami/maven/internal/config" ) type nexusClient struct { @@ -49,6 +49,26 @@ type nexusResolveResult struct { Entity *nexusEntity `json:"entity,omitempty"` Score float64 `json:"score,omitempty"` Candidates []nexusCandidate `json:"candidates,omitempty"` + + // Flat fields per ECOSYSTEM-SPEC.md §1.5's documented resolve response + // shape. Nexus emits both this and the nested Entity above; normalize + // into Entity in UnmarshalJSON so callers only ever look at one place. + EntityID string `json:"entity_id,omitempty"` + EntityType string `json:"entity_type,omitempty"` + DisplayName string `json:"display_name,omitempty"` +} + +func (r *nexusResolveResult) UnmarshalJSON(data []byte) error { + type alias nexusResolveResult + var a alias + if err := json.Unmarshal(data, &a); err != nil { + return err + } + *r = nexusResolveResult(a) + if r.Entity == nil && r.EntityID != "" { + r.Entity = &nexusEntity{ID: r.EntityID, Type: r.EntityType, DisplayName: r.DisplayName} + } + return nil } func (c *nexusClient) Resolve(ctx context.Context, query string, types []string) (*nexusResolveResult, error) { @@ -207,16 +227,21 @@ func (w *ecosystemWiring) resolveEntityReference(ctx context.Context, text strin } // discoverCapabilities returns Hexis capabilities applicable to an entity. -func (w *ecosystemWiring) discoverCapabilities(ctx context.Context, entityID string) []hexisclient.Capability { +// A non-nil error means Hexis could not be reached or refused the request — +// distinct from a nil error with zero capabilities, which means Hexis is +// healthy and genuinely has nothing registered for this entity. Callers must +// not conflate the two: a dependency failure must not silently read as "no +// capabilities" and fall through to unrelated local execution. +func (w *ecosystemWiring) discoverCapabilities(ctx context.Context, entityID string) ([]hexisclient.Capability, error) { if w == nil || w.hexis == nil || entityID == "" { - return nil + return nil, nil } caps, err := w.hexis.Capabilities(ctx, entityID) if err != nil { log.Printf("ecosystem: hexis capabilities error: %v", err) - return nil + return nil, err } - return caps + return caps, nil } // executeCapability runs a Hexis capability, tagging the request with a diff --git a/cmd/mavend/ecosystem_test.go b/cmd/mavend/ecosystem_test.go index 8ffd40f..b603560 100644 --- a/cmd/mavend/ecosystem_test.go +++ b/cmd/mavend/ecosystem_test.go @@ -140,3 +140,101 @@ func TestHexisAmbiguousAsksClarification(t *testing.T) { t.Fatal("ambiguous target must never execute") } } + +// TestHexisResolveFlatShapeAccepted covers ECOSYSTEM-SPEC.md §1.5's documented +// flat resolve response (entity_id/entity_type/display_name at the top level, +// no nested entity object) alongside the nested shape Maven already decodes. +func TestHexisResolveFlatShapeAccepted(t *testing.T) { + ctx := context.Background() + flat := `{"status":"resolved","entity_id":"ent_muzick","entity_type":"service","display_name":"Muzick indexer"}` + caps := `[{"id":"cap_status","name":"restart","read_only":true}]` + h, executed := newHexisTestHandler(t, flat, caps) + + reply := h.handleHexisAct(ctx, actDec("muzick indexer")) + if !*executed { + t.Fatalf("flat-shaped resolved entity should still execute, got reply %q", reply) + } +} + +// TestHexisNexusErrorFailsClosed covers the P0 audit finding: a genuine Nexus +// dependency failure must stop the ecosystem action and report degradation, +// never silently fall through to the local system command executor. +func TestHexisNexusErrorFailsClosed(t *testing.T) { + ctx := context.Background() + nexus := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "boom", http.StatusInternalServerError) + })) + t.Cleanup(nexus.Close) + hexis := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Fatal("hexis must not be contacted when nexus resolve fails") + })) + t.Cleanup(hexis.Close) + + st := newTestStore(t) + now := time.Now() + h := &reactiveHandler{ + api: ipc.NewStoreAPI(st), + dataStore: st, + now: func() time.Time { return now }, + ecosystem: stubEcosystem(nexus.URL, hexis.URL), + } + + reply := h.handleHexisAct(ctx, actDec("muzick indexer")) + if reply == "" { + t.Fatal("nexus dependency failure must not fall through with an empty reply") + } + if strings.Contains(reply, "выполнена") { + t.Fatalf("nexus dependency failure must not report success, got %q", reply) + } +} + +// TestHexisUnavailableFailsClosed covers the same invariant for a resolved +// entity whose Hexis capability discovery then fails. +func TestHexisUnavailableFailsClosed(t *testing.T) { + ctx := context.Background() + resolved := `{"status":"resolved","entity":{"id":"ent_muzick","display_name":"Muzick indexer","type":"service"}}` + nexus := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(resolved)) + })) + t.Cleanup(nexus.Close) + hexis := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "unavailable", http.StatusServiceUnavailable) + })) + t.Cleanup(hexis.Close) + + st := newTestStore(t) + now := time.Now() + h := &reactiveHandler{ + api: ipc.NewStoreAPI(st), + dataStore: st, + now: func() time.Time { return now }, + ecosystem: stubEcosystem(nexus.URL, hexis.URL), + } + + reply := h.handleHexisAct(ctx, actDec("muzick indexer")) + if reply == "" { + t.Fatal("hexis dependency failure must not fall through with an empty reply") + } + if strings.Contains(reply, "выполнена") { + t.Fatalf("hexis dependency failure must not report success, got %q", reply) + } +} + +// TestHexisNotFoundStillFallsThrough ensures the fail-closed fix above is +// scoped to genuine dependency errors: a resolved-but-empty ("not_found") +// Nexus response — meaning the text simply isn't a known entity, not that +// Nexus is broken — must still fall through to the local command executor. +func TestHexisNotFoundStillFallsThrough(t *testing.T) { + ctx := context.Background() + notFound := `{"status":"not_found"}` + h, executed := newHexisTestHandler(t, notFound, `[]`) + + reply := h.handleHexisAct(ctx, actDec("turn off the lights")) + if reply != "" { + t.Fatalf("not_found resolution should fall through with empty reply, got %q", reply) + } + if *executed { + t.Fatal("not_found resolution must never execute a hexis capability") + } +} diff --git a/cmd/mavend/voice.go b/cmd/mavend/voice.go index 8cfe9c8..36d4708 100644 --- a/cmd/mavend/voice.go +++ b/cmd/mavend/voice.go @@ -56,6 +56,7 @@ import ( "sync" "time" + hexisclient "github.com/kami/hexis/pkg/client" "github.com/kami/maven/internal/audio" "github.com/kami/maven/internal/config" "github.com/kami/maven/internal/delivery" @@ -75,7 +76,6 @@ import ( "github.com/kami/maven/internal/voice" "github.com/kami/maven/internal/weather" "github.com/kami/maven/internal/worker" - hexisclient "github.com/kami/hexis/pkg/client" ) // voiceWiring — everything the daemon needs to run the audio path. Held by @@ -1229,7 +1229,11 @@ func (h *reactiveHandler) handleHexisAct(ctx context.Context, dec router.Decisio // ambiguous match must stop and clarify — never guess a mutation target. entityID, displayName, ambiguous, err := h.ecosystem.resolveEntityReference(ctx, dec.Slots.Text, nil) if err != nil { - return "" + // A genuine Nexus dependency failure, not "no such entity" — stop here + // and report degradation rather than silently falling through to the + // local command executor (ECOSYSTEM-SPEC.md: services degrade + // independently, never a silent all-clear). + return "экосистема недоступна, попробуй ещё раз." } if len(ambiguous) > 0 { return "уточни, что именно: " + strings.Join(ambiguous, ", ") + "?" @@ -1238,8 +1242,13 @@ func (h *reactiveHandler) handleHexisAct(ctx context.Context, dec router.Decisio return "" } - // Discover Hexis capabilities for this entity. - caps := h.ecosystem.discoverCapabilities(ctx, entityID) + // Discover Hexis capabilities for this entity. A resolved entity with a + // genuine Hexis failure must not be treated as "no capabilities" and + // fall through to unrelated local execution. + caps, err := h.ecosystem.discoverCapabilities(ctx, entityID) + if err != nil { + return "экосистема недоступна, попробуй ещё раз." + } if len(caps) == 0 { return "" }