Fail closed on Nexus/Hexis dependency errors, accept flat resolve shape

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.
2026-07-20 01:08:11 +04:00
parent 44421d2048
commit d9fa4d6613
3 changed files with 141 additions and 9 deletions
@@ -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
@@ -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")
+	}
+}
@@ -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 ""
 	}