dda4acfbb6
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uea55zaiWuEByEDC4UBSdd
208 lines
7.0 KiB
Go
208 lines
7.0 KiB
Go
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)
|
|
}
|
|
}
|