From 5d2fd91c06fb1b58d19cfd93a663836610874ab2 Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 6 Aug 2026 02:48:00 +0400 Subject: [PATCH 1/2] a Hexis 401 says the token was refused, not that Hexis is down (V-587) The vendored Hexis client is a separate implementation and returns a plain fmt.Errorf for every status at or above 400, so errors.As for *ecosystemError never matched, Unauthorized() was never consulted, and ecosystemGap always fell through to the outage line. A wrong token sent him to inspect a healthy service. hexisError classifies at Maven's boundary, since the client is vendored from another repo and a local edit there is lost on the next re-vendor. The status text is the only signal that survives the wrapping, so that is what it reads; anything unrecognised stays at status 0, which is what Unreachable() means. The correct fix is a typed error upstream carrying the code, and Maven cannot land it unilaterally. execHexis is the second site and it did not call ecosystemGap at all. It now does, but only for a failure that belongs to the service. An execution that Hexis accepted and that then failed keeps the command-level line: that is the command failing, not Hexis degrading, and calling it an outage would be the same defect pointed the other way. Authorization is unchanged: a 401 is still a refusal, it is not retried and nothing proceeds on it. Co-Authored-By: Claude Opus 5 --- cmd/mavend/ecosystem.go | 55 ++++++++++- cmd/mavend/ecosystem_acts.go | 16 ++++ cmd/mavend/ecosystem_gap_test.go | 159 +++++++++++++++++++++++++++++++ 3 files changed, 229 insertions(+), 1 deletion(-) create mode 100644 cmd/mavend/ecosystem_gap_test.go diff --git a/cmd/mavend/ecosystem.go b/cmd/mavend/ecosystem.go index 2e1a9bf..8ff69e0 100644 --- a/cmd/mavend/ecosystem.go +++ b/cmd/mavend/ecosystem.go @@ -12,6 +12,7 @@ import ( "log" "net/http" "net/url" + "strings" "time" hexisclient "github.com/kami/hexis/pkg/client" @@ -179,6 +180,54 @@ func httpError(service, op string, status int) *ecosystemError { } } +// hexisStatusTexts maps the http.StatusText spelling back to its code, for the +// failure statuses a Hexis call can plausibly answer with. It is the inverse of +// what the vendored client threw away. +var hexisStatusTexts = func() map[string]int { + codes := []int{ + http.StatusBadRequest, http.StatusUnauthorized, http.StatusForbidden, + http.StatusNotFound, http.StatusMethodNotAllowed, http.StatusNotAcceptable, + http.StatusRequestTimeout, http.StatusConflict, http.StatusGone, + http.StatusUnprocessableEntity, http.StatusUpgradeRequired, + http.StatusTooManyRequests, http.StatusInternalServerError, + http.StatusNotImplemented, http.StatusBadGateway, + http.StatusServiceUnavailable, http.StatusGatewayTimeout, + } + m := make(map[string]int, len(codes)) + for _, c := range codes { + m[http.StatusText(c)] = c + } + return m +}() + +// hexisError re-wraps an error from the vendored Hexis client as an +// *ecosystemError, so a Hexis failure classifies the same way a Nexus or Praxis +// one does and ecosystemGap can tell a refused credential from an outage. +// +// This is a boundary adapter and it is not the fix anyone would choose. The +// Hexis client lives in another repository and returns +// fmt.Errorf("%s: %s", http.StatusText(status), body) for every status at or +// above 400, so the status text is the only signal that survives — the correct +// fix is a typed error carrying the code, and Maven cannot land it unilaterally +// (Vikunja #587, docs/plans/20-two-artifacts-and-neither-is-spring.md). Parsing +// here is bounded: the message's first colon-delimited segment is the status +// text verbatim, no status text contains a colon, and anything unrecognised — +// "do request: ...", "create request: ..." — is a transport failure and is left +// at status 0, which is exactly what Unreachable() means. +func hexisError(op string, err error) error { + if err == nil { + return nil + } + var ee *ecosystemError + if errors.As(err, &ee) { + return err + } + head, _, _ := strings.Cut(err.Error(), ": ") + return &ecosystemError{ + Service: "hexis", Op: op, Status: hexisStatusTexts[head], Err: err, + } +} + type nexusClient struct { ecosystemHTTP } @@ -530,6 +579,7 @@ func (w *ecosystemWiring) discoverCapabilities(ctx context.Context, entityID str } caps, err := w.hexis.Capabilities(ctx, entityID) if err != nil { + err = hexisError("capabilities", err) log.Printf("ecosystem: hexis capabilities error: %v", err) return nil, err } @@ -557,7 +607,10 @@ func (w *ecosystemWiring) executeCapability(ctx context.Context, capabilityID, t exec, err := w.hexis.Execute(ctx, req) if err != nil { - return correlationID, fmt.Errorf("execute: %w", err) + // A classified dependency failure. The two returns below are NOT: an + // execution that ran and failed is the command failing, not Hexis + // degrading, and it keeps its plain error so the caller says so. + return correlationID, hexisError("execute", err) } if exec.Status == "succeeded" { return correlationID, nil diff --git a/cmd/mavend/ecosystem_acts.go b/cmd/mavend/ecosystem_acts.go index 4c7a852..94f41c2 100644 --- a/cmd/mavend/ecosystem_acts.go +++ b/cmd/mavend/ecosystem_acts.go @@ -551,6 +551,14 @@ func unauthorizedEcosystemError(err error) bool { return errors.As(err, &ee) && ee.Unauthorized() } +// isEcosystemError reports a failure that belongs to the service rather than to +// what was asked of it: a call that never landed, or one the far side refused. +// It separates "Hexis is down" from "the restart failed". +func isEcosystemError(err error) bool { + var ee *ecosystemError + return errors.As(err, &ee) +} + // traceErrorFields describes an ecosystemError for a trace without leaking the // payload: the HTTP status and the failure class, nothing else. func traceErrorFields(err error) map[string]any { @@ -777,6 +785,14 @@ func (h *reactiveHandler) execHexis(ctx context.Context, capID, capName, entityI mergeFields(traceErrorFields(err), map[string]any{ "entity_id": entityID, "capability": capName, "causation_id": causationID, })) + // Hexis never answering, or answering "no", is a gap in Hexis and is + // named as one — a refused token said "не получилось выполнить команду" + // here and sent him to debug a capability that was never reached + // (Vikunja #587). An execution that genuinely ran and failed is not an + // ecosystemError and keeps the command-level line. + if isEcosystemError(err) { + return ecosystemGap(serviceHexis, err) + } return phraser.A(phraser.ActFailEntity, map[string]string{"name": displayName}) } // One record per hop: the second write this used to make said the same diff --git a/cmd/mavend/ecosystem_gap_test.go b/cmd/mavend/ecosystem_gap_test.go new file mode 100644 index 0000000..e5797a0 --- /dev/null +++ b/cmd/mavend/ecosystem_gap_test.go @@ -0,0 +1,159 @@ +package main + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/kami/maven/internal/ipc" + "github.com/kami/maven/internal/phraser" +) + +// A refused credential and an outage are different answers, and on the Hexis +// path only one of them used to be said. These tests pin the difference at both +// Hexis sites: the discovery hop and the execute hop (Vikunja #587). The Praxis +// half of the same defect is in praxis_gap_test.go. +// +// unreachableURL is a port nothing listens on, which is what "the service is +// down" looks like from inside a call: the connection is refused, no HTTP +// answer is ever produced, and ecosystemError.Unreachable() is true. +const unreachableURL = "http://127.0.0.1:1" + +func denied(service, reply string) bool { + return phraser.IsA(phraser.EcoDenied, serviceVars(service), reply) +} + +func down(service, reply string) bool { + return phraser.IsA(phraser.EcoDown, serviceVars(service), reply) +} + +// hexisGapHandler wires a handler whose Nexus resolves cleanly and whose Hexis +// is the caller's to break. hexisURL is taken separately so a test can point it +// at a dead port. +func hexisGapHandler(t *testing.T, nexusURL, hexisURL string) *reactiveHandler { + t.Helper() + st := newTestStore(t) + now := time.Now() + return &reactiveHandler{ + api: ipc.NewStoreAPI(st), + dataStore: st, + now: func() time.Time { return now }, + ecosystem: stubEcosystem(nexusURL, hexisURL), + } +} + +// TestHexisDiscovery401IsDeniedNotDown — the discovery hop. +// +// The vendored Hexis client returns a plain fmt.Errorf for every status at or +// above 400, so errors.As for *ecosystemError never matched and every failure +// fell through to the outage line. "Hexis is down" for a rejected token sends +// him to inspect a service that is running fine. +func TestHexisDiscovery401IsDeniedNotDown(t *testing.T) { + ctx := context.Background() + nexus := newFakeNexus(t, fixtureNexusResolved("ent_muzick", muzickIndexer, "service")) + caps := fixtureHexisCapabilities(map[string]any{"id": "cap_restart", "name": "restart", "read_only": true}) + hexis := newFakeHexis(t, caps, fixtureHexisExecuted("exec_1", "succeeded")) + h := hexisGapHandler(t, nexus.URL, hexis.URL) + + hexis.SetFault(401) + reply := h.handleHexisAct(ctx, actDec("muzick indexer")) + if !denied(serviceHexis, reply) { + t.Fatalf("401 from hexis discovery: got %q, want the denied line naming Hexis", reply) + } + if !strings.Contains(reply, serviceHexis) { + t.Errorf("reply does not name Hexis: %q", reply) + } +} + +// TestHexisDiscoveryOutageIsDownNotDenied — the other half of the same fork. +// Without this the fix could pass by calling everything a refused credential. +func TestHexisDiscoveryOutageIsDownNotDenied(t *testing.T) { + ctx := context.Background() + nexus := newFakeNexus(t, fixtureNexusResolved("ent_muzick", muzickIndexer, "service")) + h := hexisGapHandler(t, nexus.URL, unreachableURL) + + reply := h.handleHexisAct(ctx, actDec("muzick indexer")) + if !down(serviceHexis, reply) { + t.Fatalf("connection refused from hexis: got %q, want the outage line naming Hexis", reply) + } + if denied(serviceHexis, reply) { + t.Error("an outage must not be reported as a refused credential") + } +} + +// TestHexisExecute401IsDeniedNotCommandFailure — the execute hop, which did not +// consult ecosystemGap at all and named neither the service nor the cause. +func TestHexisExecute401IsDeniedNotCommandFailure(t *testing.T) { + ctx := context.Background() + nexus := newFakeNexus(t, fixtureNexusResolved("ent_muzick", muzickIndexer, "service")) + caps := fixtureHexisCapabilities(map[string]any{"id": "cap_restart", "name": "restart", "read_only": true}) + hexis := newFakeHexis(t, caps, fixtureHexisExecuted("exec_1", "succeeded")) + h := hexisGapHandler(t, nexus.URL, hexis.URL) + + // Discovery stays healthy; only the execute endpoint refuses. A blanket + // fault would never reach the site under test. + hexis.SetRouteFault("/api/v1/execute", 401) + reply := h.handleHexisAct(ctx, actDec("muzick indexer")) + if !denied(serviceHexis, reply) { + t.Fatalf("401 from hexis execute: got %q, want the denied line naming Hexis", reply) + } +} + +// TestHexisExecuteOutageIsDown — same site, the other classification. +// +// Discovery and execution share one base URL, so the outage has to be scoped to +// the execute endpoint rather than to the server: it answers capabilities +// normally and drops the connection on execute, which is what the client sees +// when the far side dies mid-call. That produces no HTTP status at all, which is +// what Unreachable() means. +func TestHexisExecuteOutageIsDown(t *testing.T) { + ctx := context.Background() + nexus := newFakeNexus(t, fixtureNexusResolved("ent_muzick", muzickIndexer, "service")) + caps := fixtureHexisCapabilities(map[string]any{"id": "cap_restart", "name": "restart", "read_only": true}) + hexis := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/api/v1/execute" { + conn, _, err := w.(http.Hijacker).Hijack() + if err != nil { + t.Errorf("hijack: %v", err) + return + } + conn.Close() + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(caps)) + })) + t.Cleanup(hexis.Close) + h := hexisGapHandler(t, nexus.URL, hexis.URL) + + reply := h.handleHexisAct(ctx, actDec("muzick indexer")) + if !down(serviceHexis, reply) { + t.Fatalf("dropped connection on hexis execute: got %q, want the outage line", reply) + } + if denied(serviceHexis, reply) { + t.Error("an outage must not be reported as a refused credential") + } +} + +// TestHexisExecutionFailedStaysCommandFailure — the boundary of the fix. Hexis +// answering 200 with a failed execution is the command failing, not Hexis +// degrading, and it must keep the command-level line rather than accusing a +// healthy service of being down. +func TestHexisExecutionFailedStaysCommandFailure(t *testing.T) { + ctx := context.Background() + nexus := newFakeNexus(t, fixtureNexusResolved("ent_muzick", muzickIndexer, "service")) + caps := fixtureHexisCapabilities(map[string]any{"id": "cap_restart", "name": "restart", "read_only": true}) + hexis := newFakeHexis(t, caps, fixtureHexisExecutionFailed("exec_1", "unit refused to start")) + h := hexisGapHandler(t, nexus.URL, hexis.URL) + + reply := h.handleHexisAct(ctx, actDec("muzick indexer")) + if down(serviceHexis, reply) || denied(serviceHexis, reply) { + t.Fatalf("a failed execution must not be reported as an ecosystem gap, got %q", reply) + } + if !phraser.IsA(phraser.ActFailEntity, map[string]string{"name": muzickIndexer}, reply) { + t.Fatalf("want the command-failure line, got %q", reply) + } +} From 502327678f720e13176a8d702593b7329f2099bb Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 6 Aug 2026 02:49:09 +0400 Subject: [PATCH 2/2] a Praxis lifecycle failure names Praxis (V-588) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit praxisItemAction.handle returned a hardcoded per-verb constant on any error, so a Praxis outage, a refused token, a contract mismatch and a decode failure all said the same thing and none of them said "Praxis". The information already existed: praxisClient embeds ecosystemHTTP, so the error is an *ecosystemError with working classifiers, and handle() logged it, traced it and threw it away. servicePraxis joins the two service constants and the failure goes through ecosystemGap, which is what Nexus and Hexis already use. The per-verb string is kept in front of it rather than replaced: it carries which operation did not happen, and the trace is the only other place that exists. No new Russian is added — both halves are lines that already ship. Co-Authored-By: Claude Opus 5 --- cmd/mavend/ecosystem_acts.go | 15 +++++++--- cmd/mavend/praxis_gap_test.go | 54 +++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 4 deletions(-) create mode 100644 cmd/mavend/praxis_gap_test.go diff --git a/cmd/mavend/ecosystem_acts.go b/cmd/mavend/ecosystem_acts.go index 94f41c2..3f6f851 100644 --- a/cmd/mavend/ecosystem_acts.go +++ b/cmd/mavend/ecosystem_acts.go @@ -22,8 +22,9 @@ import ( // already knows which one it was talking to — it records the same name in the // trace (Vikunja #521). const ( - serviceNexus = "Nexus" - serviceHexis = "Hexis" + serviceNexus = "Nexus" + servicePraxis = "Praxis" + serviceHexis = "Hexis" ) // serviceVars — the one-key map the eco_down and eco_denied lines take. @@ -141,7 +142,13 @@ type praxisItemAction struct { verbs []string ask string // reply when no item id was given op string // trace + log name of the operation - failure string // reply when the Praxis call errors + // failure is the first half of the reply when the Praxis call errors: which + // operation did not happen. ecosystemGap supplies the second half, which + // names Praxis and splits a refused token from an outage — those two used to + // produce the identical sentence and neither said "Praxis" (Vikunja #588). + // The verb is kept alongside the service name because the trace is the only + // other place it exists, and he is not reading the trace. + failure string success string call func(ctx context.Context, px *praxisClient, id string) error } @@ -158,7 +165,7 @@ func (a praxisItemAction) handle(ctx context.Context, h *reactiveHandler, px *pr log.Printf("ecosystem: praxis %s %s: %v", a.op, id, err) h.recordEcosystemTrace(ctx, "praxis", a.op, traceStatusForError(err), started, mergeFields(traceErrorFields(err), map[string]any{"item_id": id})) - return a.failure + return a.failure + " " + ecosystemGap(servicePraxis, err) } h.recordPraxisTrace(ctx, a.op, started, map[string]any{"item_id": id}) return a.success diff --git a/cmd/mavend/praxis_gap_test.go b/cmd/mavend/praxis_gap_test.go new file mode 100644 index 0000000..51a38bd --- /dev/null +++ b/cmd/mavend/praxis_gap_test.go @@ -0,0 +1,54 @@ +package main + +import ( + "context" + "strings" + "testing" + + "github.com/kami/maven/internal/phraser" +) + +// A Praxis lifecycle failure used to return a hardcoded constant that named the +// verb and never the service, so an outage, a refused token and a contract +// mismatch all produced the identical sentence (Vikunja #588). The helpers and +// the Hexis half of the same defect are in ecosystem_gap_test.go. + +func TestPraxisLifecycle401NamesPraxis(t *testing.T) { + ctx := context.Background() + praxis := newFakePraxis(t, fixturePraxisAttentionItems()) + h := newPraxisTestHandler(t, praxis) + + praxis.SetFault(401) + reply := h.handlePraxisAct(ctx, praxisItemDec("resolve_item", "item_1")) + if !strings.Contains(reply, servicePraxis) { + t.Fatalf("praxis failure does not name Praxis: %q", reply) + } + if !strings.Contains(reply, phraser.A(phraser.EcoDenied, serviceVars(servicePraxis))) { + t.Fatalf("401 from praxis: got %q, want the denied line", reply) + } + // The verb that did not happen is still said: the trace is the only other + // place it exists and he is not reading the trace. + if !strings.Contains(reply, "не получилось отметить сделанным.") { + t.Errorf("reply dropped the operation that failed: %q", reply) + } +} + +// TestPraxisLifecycleOutageDiffersFrom401 — the identity that was the bug. +func TestPraxisLifecycleOutageDiffersFrom401(t *testing.T) { + ctx := context.Background() + praxis := newFakePraxis(t, fixturePraxisAttentionItems()) + h := newPraxisTestHandler(t, praxis) + + praxis.SetFault(401) + refused := h.handlePraxisAct(ctx, praxisItemDec("acknowledge_item", "item_1")) + + h.ecosystem = &ecosystemWiring{praxis: newPraxisClient(unreachableURL)} + outage := h.handlePraxisAct(ctx, praxisItemDec("acknowledge_item", "item_1")) + + if refused == outage { + t.Fatalf("a refused token and an outage still say the same thing: %q", refused) + } + if !strings.Contains(outage, phraser.A(phraser.EcoDown, serviceVars(servicePraxis))) { + t.Fatalf("praxis outage: got %q, want the outage line naming Praxis", outage) + } +}