From dda4acfbb6735c35c1440a6358743fb20a54676d Mon Sep 17 00:00:00 2001 From: kami Date: Thu, 30 Jul 2026 23:40:20 +0400 Subject: [PATCH] Serve capabilities through one serializer and add GET /api/v1/executions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two "refactor later" items from REVIEW-2026-07-30.md; they share the wire types, so they land together. A capability had four divergent wire shapes — the HTTP handler, the MCP adapter, pkg/client, and Maven's vendored copy of it. There is now a single definition in pkg/client, mapped from domain by internal/wire and used by the HTTP list/create/get paths and all four MCP surfaces. It lives in pkg/client rather than internal so external consumers need not vendor internal/domain, and so producer and consumer are literally the same type. The unified shape is a strict superset of all four predecessors; nothing was dropped. It adds enabled and requires_confirmation to the list responses (never omitempty — an absent bool reads as unknown, not false), capability_id to the MCP and client shapes, and the timing/attribute/version fields previously only on get-by-ID. target_types and the list itself now serialize as [] rather than null. Both `id` and `capability_id` are deliberately kept, carrying the same value. Maven decodes `id`; the spec and the rest of the API say `capability_id`. Bearer auth is already a breaking change for that consumer, and stacking a second silent one is the wrong trade — the redundancy stays until every consumer is confirmed on capability_id, then `id` goes in an announced removal. A test pins this and says so. GET /api/v1/executions?entity_id=&since=&limit= implements spec §4.5, which the Command Center needs. `since` reuses the changes-feed cursor convention rather than inventing a second paging idiom. That cursor is the row's implicit SQLite rowid, which is safe only while nothing deletes executions and nothing VACUUMs — both would renumber and silently invalidate outstanding cursors. If retention is ever added, this must become an explicit monotonic column first; the constraint is documented at the query site. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Uea55zaiWuEByEDC4UBSdd --- internal/api/capability_shape_test.go | 124 +++++++++++++ internal/api/executions_test.go | 253 ++++++++++++++++++++++++++ internal/api/handler.go | 60 +++++- internal/mcp/adapter.go | 29 +-- internal/storage/interface.go | 1 + internal/storage/sqlite.go | 93 ++++++++++ internal/wire/capability.go | 54 ++++++ internal/wire/capability_test.go | 207 +++++++++++++++++++++ pkg/client/capability.go | 62 +++++++ pkg/client/client.go | 110 +++++++---- 10 files changed, 926 insertions(+), 67 deletions(-) create mode 100644 internal/api/capability_shape_test.go create mode 100644 internal/api/executions_test.go create mode 100644 internal/wire/capability.go create mode 100644 internal/wire/capability_test.go create mode 100644 pkg/client/capability.go diff --git a/internal/api/capability_shape_test.go b/internal/api/capability_shape_test.go new file mode 100644 index 0000000..dfd64f9 --- /dev/null +++ b/internal/api/capability_shape_test.go @@ -0,0 +1,124 @@ +package api + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/kami/hexis/internal/domain" + "github.com/kami/hexis/internal/storage" +) + +func seedCapability(t *testing.T, store *storage.Store, entityID string) *domain.Capability { + t.Helper() + now := time.Now().UTC() + c := &domain.Capability{ + ID: domain.NewCapabilityID(), + Name: "docker.restart_container", + Description: "Restart a container", + TargetTypes: []string{"container"}, + TargetEntityID: entityID, + Provider: "workspace_mcp", + Operation: "docker.restart_container", + Risk: domain.RiskMedium, + ReadOnly: false, + RequiresConfirmation: domain.RequiresConfirmationForRisk(domain.RiskMedium), + Enabled: domain.EnabledForRisk(domain.RiskMedium), + TimeoutSeconds: domain.DefaultCapabilityTimeoutSeconds, + Attributes: map[string]any{}, + CreatedAt: now, + UpdatedAt: now, + Version: 1, + } + if err := store.CreateCapability(c); err != nil { + t.Fatalf("create capability: %v", err) + } + return c +} + +// TestCapabilityWireShape_ListAndGetAgree pins the unification: the list +// response and the single-capability response are now produced by the same +// serializer, so a client cannot see one shape from one endpoint and a +// different shape from the other. Before this, list emitted a hand-built map +// (no enabled/requires_confirmation) while get returned the raw domain struct. +func TestCapabilityWireShape_ListAndGetAgree(t *testing.T) { + h, store := newTestHandler(t) + c := seedCapability(t, store, "ent_alpha") + + listReq := httptest.NewRequest(http.MethodGet, "/api/v1/capabilities", nil) + listW := httptest.NewRecorder() + h.listCapabilities(listW, listReq) + if listW.Code != http.StatusOK { + t.Fatalf("list: expected 200, got %d: %s", listW.Code, listW.Body.String()) + } + var list []map[string]any + if err := json.Unmarshal(listW.Body.Bytes(), &list); err != nil { + t.Fatalf("decode list: %v", err) + } + if len(list) != 1 { + t.Fatalf("expected 1 capability, got %d", len(list)) + } + + getW := httptest.NewRecorder() + h.getCapability(getW, httptest.NewRequest(http.MethodGet, "/api/v1/capabilities/"+c.ID, nil), c.ID) + if getW.Code != http.StatusOK { + t.Fatalf("get: expected 200, got %d: %s", getW.Code, getW.Body.String()) + } + var single map[string]any + if err := json.Unmarshal(getW.Body.Bytes(), &single); err != nil { + t.Fatalf("decode get: %v", err) + } + + for k := range single { + if _, ok := list[0][k]; !ok { + t.Errorf("field %q present on GET by ID but missing from the list response", k) + } + } + for k := range list[0] { + if _, ok := single[k]; !ok { + t.Errorf("field %q present in the list response but missing from GET by ID", k) + } + } +} + +// TestListCapabilities_ExposesGuardsAndBothIDAliases pins the two wire +// decisions at the HTTP boundary: `enabled`/`requires_confirmation` are no +// longer omitted (the review finding), and `id`/`capability_id` are both +// emitted for the duration of Maven's migration. +func TestListCapabilities_ExposesGuardsAndBothIDAliases(t *testing.T) { + h, store := newTestHandler(t) + c := seedCapability(t, store, "ent_alpha") + + w := httptest.NewRecorder() + h.listCapabilities(w, httptest.NewRequest(http.MethodGet, "/api/v1/capabilities", nil)) + + var list []map[string]any + if err := json.Unmarshal(w.Body.Bytes(), &list); err != nil { + t.Fatalf("decode: %v", err) + } + got := list[0] + + if got["id"] != c.ID || got["capability_id"] != c.ID { + t.Errorf("id aliases: id=%v capability_id=%v want %q", got["id"], got["capability_id"], c.ID) + } + if enabled, ok := got["enabled"].(bool); !ok || !enabled { + t.Errorf("enabled: got %#v, want true", got["enabled"]) + } + if rc, ok := got["requires_confirmation"].(bool); !ok || !rc { + t.Errorf("requires_confirmation: got %#v, want true (risk=medium)", got["requires_confirmation"]) + } +} + +// TestListCapabilities_EmptyIsArrayNotNull — clients iterate unconditionally. +func TestListCapabilities_EmptyIsArrayNotNull(t *testing.T) { + h, _ := newTestHandler(t) + + w := httptest.NewRecorder() + h.listCapabilities(w, httptest.NewRequest(http.MethodGet, "/api/v1/capabilities", nil)) + + if body := w.Body.String(); body != "[]\n" { + t.Errorf("expected [], got %q", body) + } +} diff --git a/internal/api/executions_test.go b/internal/api/executions_test.go new file mode 100644 index 0000000..938435b --- /dev/null +++ b/internal/api/executions_test.go @@ -0,0 +1,253 @@ +package api + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strconv" + "testing" + "time" + + "github.com/kami/hexis/internal/domain" + "github.com/kami/hexis/internal/storage" +) + +// seedExecution inserts a finished execution against targetEntityID. +func seedExecution(t *testing.T, store *storage.Store, capID, targetEntityID string) *domain.Execution { + t.Helper() + now := time.Now().UTC() + e := &domain.Execution{ + ID: domain.NewExecutionID(), + CapabilityID: capID, + TargetEntityID: targetEntityID, + Status: domain.ExecutionSucceeded, + Arguments: map[string]any{}, + CreatedAt: now, + UpdatedAt: now, + } + if err := store.CreateExecution(e); err != nil { + t.Fatalf("create execution: %v", err) + } + return e +} + +func getExecutions(t *testing.T, h *Handler, query string) []*domain.Execution { + t.Helper() + req := httptest.NewRequest(http.MethodGet, "/api/v1/executions"+query, nil) + w := httptest.NewRecorder() + h.handleExecutions(w, req) + if w.Code != http.StatusOK { + t.Fatalf("GET /api/v1/executions%s: expected 200, got %d: %s", query, w.Code, w.Body.String()) + } + var out []*domain.Execution + if err := json.Unmarshal(w.Body.Bytes(), &out); err != nil { + t.Fatalf("decode response: %v", err) + } + return out +} + +// TestListExecutions_FiltersByEntity pins the entity_id filter from +// ECOSYSTEM-SPEC.md §4.5 — the Command Center renders per-entity history and +// must not be shown another entity's executions. +func TestListExecutions_FiltersByEntity(t *testing.T) { + h, store := newTestHandler(t) + + seedExecution(t, store, "cap_a", "ent_alpha") + seedExecution(t, store, "cap_b", "ent_beta") + wantAlpha := seedExecution(t, store, "cap_c", "ent_alpha") + + all := getExecutions(t, h, "") + if len(all) != 3 { + t.Fatalf("unfiltered: expected 3 executions, got %d", len(all)) + } + + alpha := getExecutions(t, h, "?entity_id=ent_alpha") + if len(alpha) != 2 { + t.Fatalf("entity_id=ent_alpha: expected 2 executions, got %d", len(alpha)) + } + for _, e := range alpha { + if e.TargetEntityID != "ent_alpha" { + t.Errorf("entity_id filter leaked execution for %s", e.TargetEntityID) + } + } + if alpha[1].ID != wantAlpha.ID { + t.Errorf("expected ascending order ending at %s, got %s", wantAlpha.ID, alpha[1].ID) + } + + if none := getExecutions(t, h, "?entity_id=ent_nonexistent"); len(none) != 0 { + t.Errorf("unknown entity: expected 0 executions, got %d", len(none)) + } +} + +// TestListExecutions_SinceIsAnExclusiveCursor pins the pagination convention: +// `since` is the same exclusive integer cursor the /api/v1/changes feed uses, +// so a client learns one paging idiom. A cursor that returned rows it had +// already seen would make the Executions surface duplicate history. +func TestListExecutions_SinceIsAnExclusiveCursor(t *testing.T) { + h, store := newTestHandler(t) + + for i := 0; i < 4; i++ { + seedExecution(t, store, "cap_a", "ent_alpha") + } + + all := getExecutions(t, h, "") + if len(all) != 4 { + t.Fatalf("expected 4 executions, got %d", len(all)) + } + for i, e := range all { + if e.Seq == 0 { + t.Fatalf("execution %d has no seq cursor", i) + } + if i > 0 && e.Seq <= all[i-1].Seq { + t.Fatalf("seq not strictly increasing: %d then %d", all[i-1].Seq, e.Seq) + } + } + + cursor := all[1].Seq + page := getExecutions(t, h, "?since="+strconv.FormatInt(cursor, 10)) + if len(page) != 2 { + t.Fatalf("since=%d: expected 2 executions, got %d", cursor, len(page)) + } + for _, e := range page { + if e.Seq <= cursor { + t.Errorf("execution seq %d should have been excluded by since=%d", e.Seq, cursor) + } + } + + // Draining the cursor yields an empty array, not null — the Command + // Center iterates the response unconditionally. + last := all[len(all)-1].Seq + if drained := getExecutions(t, h, "?since="+strconv.FormatInt(last, 10)); len(drained) != 0 { + t.Errorf("since=%d (latest): expected 0 executions, got %d", last, len(drained)) + } +} + +// TestListExecutions_CombinesEntityAndSince checks the two filters compose, +// which is the actual Command Center polling call. +func TestListExecutions_CombinesEntityAndSince(t *testing.T) { + h, store := newTestHandler(t) + + seedExecution(t, store, "cap_a", "ent_alpha") + seedExecution(t, store, "cap_b", "ent_beta") + third := seedExecution(t, store, "cap_c", "ent_alpha") + + all := getExecutions(t, h, "") + cursor := all[0].Seq + + got := getExecutions(t, h, "?entity_id=ent_alpha&since="+strconv.FormatInt(cursor, 10)) + if len(got) != 1 { + t.Fatalf("expected 1 execution, got %d", len(got)) + } + if got[0].ID != third.ID { + t.Errorf("expected %s, got %s", third.ID, got[0].ID) + } +} + +func TestListExecutions_LimitBoundsThePage(t *testing.T) { + h, store := newTestHandler(t) + + for i := 0; i < 5; i++ { + seedExecution(t, store, "cap_a", "ent_alpha") + } + + got := getExecutions(t, h, "?limit=2") + if len(got) != 2 { + t.Fatalf("limit=2: expected 2 executions, got %d", len(got)) + } +} + +func TestListExecutions_RejectsBadCursor(t *testing.T) { + h, _ := newTestHandler(t) + + for _, q := range []string{"?since=abc", "?since=-1", "?limit=0", "?limit=nope"} { + req := httptest.NewRequest(http.MethodGet, "/api/v1/executions"+q, nil) + w := httptest.NewRecorder() + h.handleExecutions(w, req) + if w.Code != http.StatusBadRequest { + t.Errorf("%s: expected 400, got %d: %s", q, w.Code, w.Body.String()) + } + } +} + +func TestListExecutions_RejectsNonGET(t *testing.T) { + h, _ := newTestHandler(t) + + req := httptest.NewRequest(http.MethodPost, "/api/v1/executions", nil) + w := httptest.NewRecorder() + h.handleExecutions(w, req) + if w.Code != http.StatusMethodNotAllowed { + t.Errorf("expected 405, got %d", w.Code) + } +} + +// TestListExecutions_RequiresBearerToken pins that the new endpoint sits +// behind the same auth as the rest of /api/v1/ rather than being routed +// around it. Execution history names every entity and capability on the box; +// it is not public. +func TestListExecutions_RequiresBearerToken(t *testing.T) { + h, store := newTestHandler(t) + seedExecution(t, store, "cap_a", "ent_alpha") + + mux := http.NewServeMux() + h.Register(mux) + + // No token at all. + w := httptest.NewRecorder() + mux.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/api/v1/executions?entity_id=ent_alpha", nil)) + if w.Code != http.StatusUnauthorized { + t.Fatalf("unauthenticated: expected 401, got %d: %s", w.Code, w.Body.String()) + } + + // Wrong token. + req := httptest.NewRequest(http.MethodGet, "/api/v1/executions", nil) + req.Header.Set("Authorization", "Bearer wrong-token") + w = httptest.NewRecorder() + mux.ServeHTTP(w, req) + if w.Code != http.StatusUnauthorized { + t.Fatalf("wrong token: expected 401, got %d", w.Code) + } + + // Correct token. + req = httptest.NewRequest(http.MethodGet, "/api/v1/executions", nil) + req.Header.Set("Authorization", "Bearer "+testToken) + w = httptest.NewRecorder() + mux.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("authenticated: expected 200, got %d: %s", w.Code, w.Body.String()) + } + + var out []*domain.Execution + if err := json.Unmarshal(w.Body.Bytes(), &out); err != nil { + t.Fatalf("decode: %v", err) + } + if len(out) != 1 { + t.Fatalf("expected 1 execution, got %d", len(out)) + } +} + +// TestListExecutions_RoutedSeparatelyFromGetByID guards the ServeMux split: +// "/api/v1/executions" (list) and "/api/v1/executions/" (by ID) are distinct +// patterns, and adding the former must not have shadowed the latter. +func TestListExecutions_RoutedSeparatelyFromGetByID(t *testing.T) { + h, store := newTestHandler(t) + exec := seedExecution(t, store, "cap_a", "ent_alpha") + + mux := http.NewServeMux() + h.Register(mux) + + req := httptest.NewRequest(http.MethodGet, "/api/v1/executions/"+exec.ID, nil) + req.Header.Set("Authorization", "Bearer "+testToken) + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("get by id: expected 200, got %d: %s", w.Code, w.Body.String()) + } + + var single domain.Execution + if err := json.Unmarshal(w.Body.Bytes(), &single); err != nil { + t.Fatalf("decode: %v", err) + } + if single.ID != exec.ID { + t.Errorf("expected %s, got %s", exec.ID, single.ID) + } +} diff --git a/internal/api/handler.go b/internal/api/handler.go index 85d130f..8938b98 100644 --- a/internal/api/handler.go +++ b/internal/api/handler.go @@ -206,7 +206,7 @@ func (h *Handler) createCapability(w http.ResponseWriter, r *http.Request) { Payload: map[string]any{"capability_id": cap.ID, "name": cap.Name}, }) - writeJSON(w, http.StatusCreated, cap) + writeJSON(w, http.StatusCreated, wire.Capability(cap)) } func (h *Handler) handleCapabilityByID(w http.ResponseWriter, r *http.Request) { @@ -232,7 +232,7 @@ func (h *Handler) getCapability(w http.ResponseWriter, r *http.Request, id strin writeJSON(w, http.StatusNotFound, errorResponse(err.Error())) return } - writeJSON(w, http.StatusOK, cap) + writeJSON(w, http.StatusOK, wire.Capability(cap)) } func (h *Handler) deleteCapability(w http.ResponseWriter, r *http.Request, id string) { @@ -327,19 +327,61 @@ func (h *Handler) handleConfirmations(w http.ResponseWriter, r *http.Request) { conf, err := h.engine.CreateConfirmation(req.CapabilityID, req.TargetEntityID, req.Requester, req.Arguments) if err != nil { - status := http.StatusBadRequest - if errors.Is(err, domain.ErrCapabilityNotFound) { - status = http.StatusNotFound - } else if errors.Is(err, domain.ErrCapabilityNotBound) { - status = http.StatusForbidden - } - writeJSON(w, status, errorResponse(err.Error())) + writeJSON(w, executeErrorStatus(err), errorResponse(err.Error())) return } writeJSON(w, http.StatusCreated, conf) } +// handleExecutions serves GET /api/v1/executions?entity_id=&since=&limit= +// (ECOSYSTEM-SPEC.md §4.5) — the execution history the Command Center's +// Overview and Executions surfaces render. +// +// `since` follows the same cursor convention as /api/v1/changes: an integer +// sequence, exclusive, with results ordered ascending. Callers page by passing +// the `seq` of the last execution they saw. This deliberately reuses the +// changes-feed style rather than introducing a timestamp cursor, so a client +// only has to learn one paging idiom against Hexis. +func (h *Handler) handleExecutions(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + writeJSON(w, http.StatusMethodNotAllowed, errorResponse("method not allowed")) + return + } + + q := r.URL.Query() + + var since int64 + if s := q.Get("since"); s != "" { + parsed, err := strconv.ParseInt(s, 10, 64) + if err != nil || parsed < 0 { + writeJSON(w, http.StatusBadRequest, errorResponse("since must be a non-negative integer sequence")) + return + } + since = parsed + } + + limit := storage.MaxExecutionPageSize + if l := q.Get("limit"); l != "" { + parsed, err := strconv.Atoi(l) + if err != nil || parsed <= 0 { + writeJSON(w, http.StatusBadRequest, errorResponse("limit must be a positive integer")) + return + } + limit = parsed + } + + execs, err := h.store.ListExecutions(q.Get("entity_id"), since, limit) + if err != nil { + writeJSON(w, http.StatusInternalServerError, errorResponse(err.Error())) + return + } + if execs == nil { + execs = []*domain.Execution{} + } + writeJSON(w, http.StatusOK, execs) +} + func (h *Handler) handleExecutionByID(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodGet { writeJSON(w, http.StatusMethodNotAllowed, errorResponse("method not allowed")) diff --git a/internal/mcp/adapter.go b/internal/mcp/adapter.go index 21cf3df..f1a4f19 100644 --- a/internal/mcp/adapter.go +++ b/internal/mcp/adapter.go @@ -14,6 +14,7 @@ import ( "github.com/kami/hexis/internal/execution" "github.com/kami/hexis/internal/nexusclient" "github.com/kami/hexis/internal/storage" + "github.com/kami/hexis/internal/wire" ) type Adapter struct { @@ -145,22 +146,12 @@ func (a *Adapter) handleListCapabilities(ctx context.Context, req mcp.CallToolRe return mcp.NewToolResultError(fmt.Sprintf("list capabilities: %v", err)), nil } - var result []map[string]any - for _, c := range caps { - result = append(result, map[string]any{ - "id": c.ID, - "name": c.Name, - "description": c.Description, - "target_types": c.TargetTypes, - "target_entity_id": c.TargetEntityID, - "provider": c.Provider, - "operation": c.Operation, - "risk": c.Risk, - "read_only": c.ReadOnly, - }) - } - - data, _ := json.MarshalIndent(result, "", " ") + // Same serializer as the HTTP API, so an MCP client and an HTTP client see + // an identical capability — including `enabled` and + // `requires_confirmation`, which this tool previously hid. Tool + // availability is not permission (spec §4.5), but a client that can see + // the guard fields can at least explain a 403 instead of guessing. + data, _ := json.MarshalIndent(wire.Capabilities(caps), "", " ") return &mcp.CallToolResult{ Content: []mcp.Content{ mcp.TextContent{Type: "text", Text: string(data)}, @@ -179,7 +170,7 @@ func (a *Adapter) handleInspectCapability(ctx context.Context, req mcp.CallToolR return mcp.NewToolResultError(fmt.Sprintf("capability not found: %v", err)), nil } - data, _ := json.MarshalIndent(cap, "", " ") + data, _ := json.MarshalIndent(wire.Capability(cap), "", " ") return &mcp.CallToolResult{ Content: []mcp.Content{ mcp.TextContent{Type: "text", Text: string(data)}, @@ -317,7 +308,7 @@ func (a *Adapter) handleCapabilitiesResource(ctx context.Context, req mcp.ReadRe if err != nil { return nil, err } - data, _ := json.MarshalIndent(caps, "", " ") + data, _ := json.MarshalIndent(wire.Capabilities(caps), "", " ") return []mcp.ResourceContents{ mcp.TextResourceContents{ URI: "hexis://capabilities", @@ -339,7 +330,7 @@ func (a *Adapter) handleCapabilityResourceTemplate(ctx context.Context, req mcp. return nil, fmt.Errorf("capability %s: %w", id, err) } - data, _ := json.MarshalIndent(cap, "", " ") + data, _ := json.MarshalIndent(wire.Capability(cap), "", " ") return []mcp.ResourceContents{ mcp.TextResourceContents{ URI: uri, diff --git a/internal/storage/interface.go b/internal/storage/interface.go index 1a67e40..bf329cf 100644 --- a/internal/storage/interface.go +++ b/internal/storage/interface.go @@ -15,6 +15,7 @@ type Interface interface { UpdateExecution(e *domain.Execution) error GetExecutionByIdempotencyKey(key string) (*domain.Execution, error) GetInFlightExecution(capabilityID, targetEntityID string) (*domain.Execution, error) + ListExecutions(entityID string, sinceSeq int64, limit int) ([]*domain.Execution, error) CreateConfirmation(c *domain.Confirmation) error GetConfirmation(id string) (*domain.Confirmation, error) diff --git a/internal/storage/sqlite.go b/internal/storage/sqlite.go index a02e68a..d691b60 100644 --- a/internal/storage/sqlite.go +++ b/internal/storage/sqlite.go @@ -467,14 +467,107 @@ func (s *Store) getExecutionLocked(id string) (*domain.Execution, error) { json.Unmarshal([]byte(result), &e.Result) json.Unmarshal([]byte(evidence), &e.ResolutionEvidence) e.IdempotencyKey = idempKey + e.ConfirmationID = confID e.Status = domain.ExecutionStatus(status) e.Error = errStr e.CorrelationID = corrID + e.CausationID = causID e.CreatedAt = parseTime(createdAt) e.UpdatedAt = parseTime(updatedAt) return e, nil } +// executionColumns is the shared SELECT list for execution reads. `rowid` is +// the insertion sequence and doubles as the pagination cursor for +// ListExecutions; single-row reads ignore it. +// +// Using rowid as the cursor is safe here because executions is an ordinary +// rowid table (its PRIMARY KEY is TEXT, so rowid is a separate hidden counter) +// and nothing in this service deletes execution rows or runs VACUUM — both of +// which could renumber rowids and invalidate outstanding cursors. If either +// ever becomes true, promote this to an explicit monotonic seq column. +const executionColumns = `rowid, id, capability_id, capability_version, target_entity_id, entity_version, arguments, requested_by, origin, COALESCE(idempotency_key,''), COALESCE(confirmation_id,''), status, result, COALESCE(error,''), resolution_evidence, COALESCE(correlation_id,''), COALESCE(causation_id,''), created_at, updated_at` + +// scanExecution reads one row selected with executionColumns. +func scanExecution(sc interface{ Scan(...any) error }) (*domain.Execution, error) { + e := &domain.Execution{} + var args, reqBy, origin, idempKey, confID, status, result, errStr, evidence, corrID, causID, createdAt, updatedAt string + err := sc.Scan(&e.Seq, &e.ID, &e.CapabilityID, &e.CapabilityVersion, &e.TargetEntityID, &e.EntityVersion, &args, &reqBy, &origin, &idempKey, &confID, &status, &result, &errStr, &evidence, &corrID, &causID, &createdAt, &updatedAt) + if err != nil { + return nil, err + } + json.Unmarshal([]byte(args), &e.Arguments) + json.Unmarshal([]byte(reqBy), &e.RequestedBy) + json.Unmarshal([]byte(origin), &e.Origin) + json.Unmarshal([]byte(result), &e.Result) + json.Unmarshal([]byte(evidence), &e.ResolutionEvidence) + e.IdempotencyKey = idempKey + e.ConfirmationID = confID + e.Status = domain.ExecutionStatus(status) + e.Error = errStr + e.CorrelationID = corrID + e.CausationID = causID + e.CreatedAt = parseTime(createdAt) + e.UpdatedAt = parseTime(updatedAt) + if e.Arguments == nil { + e.Arguments = map[string]any{} + } + if e.RequestedBy == nil { + e.RequestedBy = map[string]string{} + } + if e.Origin == nil { + e.Origin = map[string]string{} + } + if e.Result == nil { + e.Result = map[string]any{} + } + return e, nil +} + +// MaxExecutionPageSize bounds a single ListExecutions page, matching the +// 100-row cap the changes feed uses. +const MaxExecutionPageSize = 100 + +// ListExecutions returns executions ordered by ascending insertion sequence. +// +// entityID, when non-empty, restricts to that target entity. sinceSeq is an +// exclusive cursor: only rows with Seq > sinceSeq are returned, the same +// convention as EventsAfter on /api/v1/changes. limit is clamped to +// MaxExecutionPageSize. +func (s *Store) ListExecutions(entityID string, sinceSeq int64, limit int) ([]*domain.Execution, error) { + s.mu.RLock() + defer s.mu.RUnlock() + + if limit <= 0 || limit > MaxExecutionPageSize { + limit = MaxExecutionPageSize + } + + query := `SELECT ` + executionColumns + ` FROM executions WHERE rowid > ?` + args := []any{sinceSeq} + if entityID != "" { + query += ` AND target_entity_id = ?` + args = append(args, entityID) + } + query += ` ORDER BY rowid ASC LIMIT ?` + args = append(args, limit) + + rows, err := s.db.Query(query, args...) + if err != nil { + return nil, err + } + defer rows.Close() + + out := []*domain.Execution{} + for rows.Next() { + e, err := scanExecution(rows) + if err != nil { + return nil, err + } + out = append(out, e) + } + return out, rows.Err() +} + // Confirmation operations func (s *Store) CreateConfirmation(c *domain.Confirmation) error { diff --git a/internal/wire/capability.go b/internal/wire/capability.go new file mode 100644 index 0000000..9253953 --- /dev/null +++ b/internal/wire/capability.go @@ -0,0 +1,54 @@ +// Package wire converts internal domain objects into the public wire shapes +// defined in pkg/client. It is the single serialization point: the HTTP +// handler and the MCP adapter both go through it, so a capability looks the +// same on every surface Hexis exposes. +package wire + +import ( + "github.com/kami/hexis/internal/domain" + "github.com/kami/hexis/pkg/client" +) + +// Capability converts a domain capability into the public wire shape. +// +// Both `capability_id` and `id` are populated with the same value; see the +// compatibility note on client.Capability for why the alias is retained. +func Capability(c *domain.Capability) client.Capability { + if c == nil { + return client.Capability{} + } + targetTypes := c.TargetTypes + if targetTypes == nil { + targetTypes = []string{} + } + return client.Capability{ + CapabilityID: c.ID, + ID: c.ID, + Name: c.Name, + Description: c.Description, + TargetTypes: targetTypes, + TargetEntityID: c.TargetEntityID, + Provider: c.Provider, + Operation: c.Operation, + Risk: c.Risk, + ReadOnly: c.ReadOnly, + ExpectedSideEffects: c.ExpectedSideEffects, + RequiresConfirmation: c.RequiresConfirmation, + Enabled: c.Enabled, + TimeoutSeconds: c.TimeoutSeconds, + Attributes: c.Attributes, + CreatedAt: c.CreatedAt, + UpdatedAt: c.UpdatedAt, + Version: c.Version, + } +} + +// Capabilities converts a slice, never returning nil so the JSON encoding is +// `[]` rather than `null`. +func Capabilities(caps []*domain.Capability) []client.Capability { + out := make([]client.Capability, 0, len(caps)) + for _, c := range caps { + out = append(out, Capability(c)) + } + return out +} diff --git a/internal/wire/capability_test.go b/internal/wire/capability_test.go new file mode 100644 index 0000000..5cfbc69 --- /dev/null +++ b/internal/wire/capability_test.go @@ -0,0 +1,207 @@ +package wire + +import ( + "encoding/json" + "testing" + "time" + + "github.com/kami/hexis/internal/domain" + "github.com/kami/hexis/pkg/client" +) + +func sampleCapability() *domain.Capability { + now := time.Date(2026, 7, 30, 12, 0, 0, 0, time.UTC) + return &domain.Capability{ + ID: "cap_docker_restart", + Name: "docker.restart_container", + Description: "Restart a container", + TargetTypes: []string{"container"}, + TargetEntityID: "", + Provider: "workspace_mcp", + Operation: "docker.restart_container", + Risk: domain.RiskMedium, + ReadOnly: false, + ExpectedSideEffects: "container restarts", + RequiresConfirmation: true, + Enabled: true, + TimeoutSeconds: 30, + Attributes: map[string]any{"allowlist": "docker"}, + CreatedAt: now, + UpdatedAt: now, + Version: 1, + } +} + +func marshalCapability(t *testing.T, c *domain.Capability) map[string]any { + t.Helper() + data, err := json.Marshal(Capability(c)) + if err != nil { + t.Fatalf("marshal: %v", err) + } + var out map[string]any + if err := json.Unmarshal(data, &out); err != nil { + t.Fatalf("unmarshal: %v", err) + } + return out +} + +// TestCapability_EmitsBothIDAliases pins the deliberate redundancy documented +// on client.Capability. Maven's consumer reads `id`; the spec and the rest of +// the API say `capability_id`. Hexis is already breaking that consumer with +// bearer auth, so dropping either alias here would be a second, silent break. +// If this test is ever changed, it must be as an announced removal, not a +// tidy-up. +func TestCapability_EmitsBothIDAliases(t *testing.T) { + out := marshalCapability(t, sampleCapability()) + + id, hasID := out["id"] + capID, hasCapID := out["capability_id"] + if !hasID { + t.Error("`id` missing — Maven's vendored client decodes this field") + } + if !hasCapID { + t.Error("`capability_id` missing — this is the ECOSYSTEM-SPEC.md §4.1 name") + } + if id != capID { + t.Errorf("aliases disagree: id=%v capability_id=%v", id, capID) + } + if id != "cap_docker_restart" { + t.Errorf("unexpected id %v", id) + } +} + +// TestCapability_CarriesServerDerivedGuards pins the review finding that +// listCapabilities omitted `enabled` and `requires_confirmation`, leaving a +// client unable to distinguish a callable capability from one that would be +// refused with 403. Both are always present, even when false. +func TestCapability_CarriesServerDerivedGuards(t *testing.T) { + out := marshalCapability(t, sampleCapability()) + + if got, ok := out["enabled"].(bool); !ok || !got { + t.Errorf("enabled: expected true, got %#v", out["enabled"]) + } + if got, ok := out["requires_confirmation"].(bool); !ok || !got { + t.Errorf("requires_confirmation: expected true, got %#v", out["requires_confirmation"]) + } + + // Both must be emitted even at their zero value — `omitempty` here would + // read to a client as "unknown", not "false". + c := sampleCapability() + c.Enabled = false + c.RequiresConfirmation = false + out = marshalCapability(t, c) + if _, ok := out["enabled"]; !ok { + t.Error("`enabled` omitted when false; it must always be present") + } + if _, ok := out["requires_confirmation"]; !ok { + t.Error("`requires_confirmation` omitted when false; it must always be present") + } +} + +// TestCapability_IsASupersetOfEveryPreviousShape guards against a silent field +// removal for consumers of any of the four shapes this serializer replaced: +// the HTTP list response, the HTTP get response (raw domain.Capability), the +// MCP adapter's map, and pkg/client. +func TestCapability_IsASupersetOfEveryPreviousShape(t *testing.T) { + out := marshalCapability(t, sampleCapability()) + + required := []string{ + // union of the old listCapabilities map and the MCP adapter map + "id", "capability_id", "name", "description", "target_types", + "provider", "operation", "risk", "read_only", "expected_side_effects", + // previously only on the raw domain.Capability returned by GET by ID + "requires_confirmation", "enabled", "timeout_seconds", "attributes", + "created_at", "updated_at", "version", + } + for _, k := range required { + if _, ok := out[k]; !ok { + t.Errorf("field %q dropped by the unified serializer", k) + } + } +} + +// TestCapability_TargetTypesNeverNull — clients range over this array; `null` +// is a decode hazard for the non-Go consumers the Command Center will add. +func TestCapability_TargetTypesNeverNull(t *testing.T) { + c := sampleCapability() + c.TargetTypes = nil + out := marshalCapability(t, c) + + tt, ok := out["target_types"].([]any) + if !ok { + t.Fatalf("target_types is %#v, want []", out["target_types"]) + } + if len(tt) != 0 { + t.Errorf("expected empty array, got %v", tt) + } +} + +// TestCapabilities_EmptySliceIsNotNull — the HTTP list endpoint must render +// `[]`, not `null`, when an entity has no capabilities. +func TestCapabilities_EmptySliceIsNotNull(t *testing.T) { + data, err := json.Marshal(Capabilities(nil)) + if err != nil { + t.Fatalf("marshal: %v", err) + } + if string(data) != "[]" { + t.Errorf("expected [], got %s", string(data)) + } +} + +// TestCapability_RoundTripsThroughPublicClient closes the loop: what the +// server serializes is exactly what pkg/client — and therefore Maven — +// decodes. A drift between producer and consumer shape is the whole class of +// bug this unification exists to remove. +func TestCapability_RoundTripsThroughPublicClient(t *testing.T) { + src := sampleCapability() + data, err := json.Marshal(Capability(src)) + if err != nil { + t.Fatalf("marshal: %v", err) + } + + var got client.Capability + if err := json.Unmarshal(data, &got); err != nil { + t.Fatalf("decode into client.Capability: %v", err) + } + + if got.ID != src.ID || got.CapabilityID != src.ID { + t.Errorf("id round-trip: ID=%q CapabilityID=%q want %q", got.ID, got.CapabilityID, src.ID) + } + if got.EffectiveID() != src.ID { + t.Errorf("EffectiveID() = %q, want %q", got.EffectiveID(), src.ID) + } + if got.Name != src.Name || got.ReadOnly != src.ReadOnly || got.Risk != src.Risk { + t.Errorf("field drift: %+v", got) + } + if !got.Enabled || !got.RequiresConfirmation { + t.Errorf("guard fields lost in round trip: %+v", got) + } + if got.Version != src.Version || !got.CreatedAt.Equal(src.CreatedAt) { + t.Errorf("metadata lost in round trip: %+v", got) + } +} + +// TestCapability_EffectiveIDToleratesEitherAlias covers a peer (an older Hexis +// or a hand-rolled client) that sends only one of the two names. +func TestCapability_EffectiveIDToleratesEitherAlias(t *testing.T) { + cases := map[string]string{ + `{"id":"cap_x"}`: "cap_x", + `{"capability_id":"cap_y"}`: "cap_y", + `{"id":"cap_z","capability_id":"cap_z"}`: "cap_z", + } + for body, want := range cases { + var c client.Capability + if err := json.Unmarshal([]byte(body), &c); err != nil { + t.Fatalf("decode %s: %v", body, err) + } + if got := c.EffectiveID(); got != want { + t.Errorf("%s: EffectiveID() = %q, want %q", body, got, want) + } + } +} + +func TestCapability_NilIsZeroValue(t *testing.T) { + if got := Capability(nil); got.EffectiveID() != "" { + t.Errorf("nil capability produced %+v", got) + } +} diff --git a/pkg/client/capability.go b/pkg/client/capability.go new file mode 100644 index 0000000..99ff964 --- /dev/null +++ b/pkg/client/capability.go @@ -0,0 +1,62 @@ +package client + +import "time" + +// Capability is THE wire shape for a Hexis capability. +// +// There is exactly one definition of it, here, and every producer in this +// repository serializes through it: the HTTP handler (GET/POST +// /api/v1/capabilities, GET /api/v1/capabilities/{id}), the MCP adapter +// (hexis.list_capabilities), and this client's decode path. It lives in +// pkg/client rather than internal/ so that external consumers get the shape +// without vendoring internal packages; internal/wire holds the +// domain.Capability -> Capability conversion. +// +// Compatibility note — `id` and `capability_id` are BOTH emitted, deliberately. +// They always carry the same value. Maven's vendored consumer decodes `id` +// (cmd/mavend/voice.go matches on Capability.ID); the ECOSYSTEM-SPEC.md §4.1 +// schema and the rest of the Hexis API name the column `capability_id`. Hexis +// is mid-rollout of bearer auth on /api/v1/, which is already one breaking +// change for that consumer; dropping either alias here would stack a second, +// silent one on top. Both stay until every consumer is confirmed to read +// `capability_id`, at which point `id` can be removed in a deliberate, +// announced change. Do not "clean this up" incidentally. +type Capability struct { + // CapabilityID is the canonical field (ECOSYSTEM-SPEC.md §4.1). + CapabilityID string `json:"capability_id"` + // ID is a deprecated alias for CapabilityID, kept for wire compatibility. + // Always identical to CapabilityID. Prefer CapabilityID in new code. + ID string `json:"id"` + + Name string `json:"name"` + Description string `json:"description,omitempty"` + TargetTypes []string `json:"target_types"` + TargetEntityID string `json:"target_entity_id,omitempty"` + Provider string `json:"provider"` + Operation string `json:"operation"` + Risk string `json:"risk,omitempty"` + ReadOnly bool `json:"read_only"` + ExpectedSideEffects string `json:"expected_side_effects,omitempty"` + + // RequiresConfirmation and Enabled are server-derived from the risk tier + // and are never settable by a caller. listCapabilities used to omit both, + // which left clients unable to tell a callable capability from one that + // would be rejected with 403; the unified shape always carries them. + RequiresConfirmation bool `json:"requires_confirmation"` + Enabled bool `json:"enabled"` + TimeoutSeconds int `json:"timeout_seconds,omitempty"` + + Attributes map[string]any `json:"attributes,omitempty"` + CreatedAt time.Time `json:"created_at,omitempty"` + UpdatedAt time.Time `json:"updated_at,omitempty"` + Version int64 `json:"version,omitempty"` +} + +// EffectiveID returns the capability ID, tolerating a peer that sends only one +// of the two aliases. +func (c Capability) EffectiveID() string { + if c.CapabilityID != "" { + return c.CapabilityID + } + return c.ID +} diff --git a/pkg/client/client.go b/pkg/client/client.go index ec06047..41dda1c 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -7,6 +7,8 @@ import ( "fmt" "io" "net/http" + "net/url" + "strconv" "time" ) @@ -42,6 +44,7 @@ func causationIDFrom(ctx context.Context) string { type Client struct { baseURL string httpClient *http.Client + token string } func New(baseURL string) *Client { @@ -51,6 +54,12 @@ func New(baseURL string) *Client { } } +// WithToken sets the shared bearer token sent on every /api/v1/ request. +func (c *Client) WithToken(token string) *Client { + c.token = token + return c +} + func (c *Client) do(ctx context.Context, method, path string, body, result any) error { var reqBody io.Reader if body != nil { @@ -67,6 +76,9 @@ func (c *Client) do(ctx context.Context, method, path string, body, result any) } req.Header.Set("Content-Type", "application/json") req.Header.Set("X-Hexis-Version", APIVersion) + if c.token != "" { + req.Header.Set("Authorization", "Bearer "+c.token) + } if id := correlationIDFrom(ctx); id != "" { req.Header.Set("X-Correlation-ID", id) } @@ -97,8 +109,37 @@ func (c *Client) do(ctx context.Context, method, path string, body, result any) return nil } -type Capability struct { - ID string `json:"id"` +// Capability is defined in capability.go — the single wire shape shared by +// the HTTP handler, the MCP adapter and this client. + +type Execution struct { + // Seq is the pagination cursor for Executions; see the `since` parameter. + // Zero on single-execution reads. + Seq int64 `json:"seq,omitempty"` + ID string `json:"id"` + CapabilityID string `json:"capability_id"` + TargetEntityID string `json:"target_entity_id"` + Status string `json:"status"` + Result map[string]any `json:"result,omitempty"` + Error string `json:"error,omitempty"` + RequestedBy map[string]string `json:"requested_by,omitempty"` + CorrelationID string `json:"correlation_id,omitempty"` + CausationID string `json:"causation_id,omitempty"` + IdempotencyKey string `json:"idempotency_key,omitempty"` +} + +type ExecuteRequest struct { + CapabilityID string `json:"capability_id"` + TargetEntityID string `json:"target_entity_id"` + Arguments map[string]any `json:"arguments,omitempty"` + RequestedBy map[string]string `json:"requested_by,omitempty"` + Origin map[string]string `json:"origin,omitempty"` + IdempotencyKey string `json:"idempotency_key,omitempty"` + CorrelationID string `json:"correlation_id,omitempty"` + CausationID string `json:"causation_id,omitempty"` +} + +type CreateCapabilityRequest struct { Name string `json:"name"` Description string `json:"description,omitempty"` TargetTypes []string `json:"target_types"` @@ -110,47 +151,11 @@ type Capability struct { ExpectedSideEffects string `json:"expected_side_effects,omitempty"` } -type Execution struct { - ID string `json:"id"` - CapabilityID string `json:"capability_id"` - TargetEntityID string `json:"target_entity_id"` - Status string `json:"status"` - Result map[string]any `json:"result,omitempty"` - Error string `json:"error,omitempty"` - RequestedBy map[string]string `json:"requested_by,omitempty"` - CorrelationID string `json:"correlation_id,omitempty"` - CausationID string `json:"causation_id,omitempty"` - IdempotencyKey string `json:"idempotency_key,omitempty"` -} - -type ExecuteRequest struct { - CapabilityID string `json:"capability_id"` - TargetEntityID string `json:"target_entity_id"` - Arguments map[string]any `json:"arguments,omitempty"` - RequestedBy map[string]string `json:"requested_by,omitempty"` - Origin map[string]string `json:"origin,omitempty"` - IdempotencyKey string `json:"idempotency_key,omitempty"` - CorrelationID string `json:"correlation_id,omitempty"` - CausationID string `json:"causation_id,omitempty"` -} - -type CreateCapabilityRequest struct { - Name string `json:"name"` - Description string `json:"description,omitempty"` - TargetTypes []string `json:"target_types"` - TargetEntityID string `json:"target_entity_id,omitempty"` - Provider string `json:"provider"` - Operation string `json:"operation"` - Risk string `json:"risk,omitempty"` - ReadOnly bool `json:"read_only"` - ExpectedSideEffects string `json:"expected_side_effects,omitempty"` -} - func (c *Client) Capabilities(ctx context.Context, entityID string) ([]Capability, error) { var result []Capability path := "/api/v1/capabilities" if entityID != "" { - path += "?entity_id=" + entityID + path += "?entity_id=" + url.QueryEscape(entityID) } if err := c.do(ctx, http.MethodGet, path, nil, &result); err != nil { return nil, err @@ -190,6 +195,33 @@ func (c *Client) GetExecution(ctx context.Context, id string) (*Execution, error return &result, nil } +// Executions returns execution history, newest last, ordered by ascending +// `seq` (ECOSYSTEM-SPEC.md §4.5). +// +// entityID, when non-empty, filters to executions against that target entity. +// since is an exclusive cursor: pass 0 for the first page, then the Seq of the +// last element returned. The server caps a page at 100 rows, so a full page +// means "call again with the new cursor". +func (c *Client) Executions(ctx context.Context, entityID string, since int64) ([]Execution, error) { + q := url.Values{} + if entityID != "" { + q.Set("entity_id", entityID) + } + if since > 0 { + q.Set("since", strconv.FormatInt(since, 10)) + } + path := "/api/v1/executions" + if len(q) > 0 { + path += "?" + q.Encode() + } + + var result []Execution + if err := c.do(ctx, http.MethodGet, path, nil, &result); err != nil { + return nil, err + } + return result, nil +} + func (c *Client) Health(ctx context.Context) error { return c.do(ctx, http.MethodGet, "/health", nil, nil) }