# Hexis — Senior Engineering Review Date: 2026-07-30 Reviewed at: commit `945e4ba` (working tree had one uncommitted change in `cmd/hexisd/main.go`: default Nexus URL `8987` → `9740`) Reference contract: `/home/kami/apps/ECOSYSTEM-SPEC.md` §4 --- ## verdict **Fragile + misaligned, but structurally sound and worth repairing incrementally.** No rewrite. The architecture matches the spec's shape; the *guards the spec exists to enforce* are missing, and the primary execution path is functionally dead at HEAD. --- ## first assessment ```text purpose: capability registry + guarded execution for the Nexus/Praxis/Hexis ecosystem (ECOSYSTEM-SPEC.md §4) intended users: Maven (voice + mavweb), Command Center, MCP clients actual users: Maven only — mavweb reads /api/v1/capabilities; mavend/voice.go:1329-1381 resolves→matches→executes via a *vendored* copy of pkg/client critical workflows: 1. list capabilities for an entity 2. confirm 3. execute 4. poll changes current state: running on :9741 — but the live binary predates commit 89d8433 (its GET capability response has no `enabled`/`requires_confirmation` fields), so deployed ≠ HEAD known failures: every workspace_mcp capability is registered with enabled=false (verified) maintenance burden: low — 3.6k LOC, 1 real dependency + pure-Go sqlite, builds clean, vet clean technical constraints: single-user homesrv, single SQLite writer, distroless container personal constraints: solo maintainer; spec is the contract, three services must agree what still works well: confirmation lifecycle (args_hash + version + TTL) is correct and genuinely well tested; storage layer is honest; timeout⇒unknown is right what has become obsolete: systemd provider (no systemctl in distroless), ListenUnix, the contract_test.go "bug documentation" tests, schema_migrations table ``` Classification: **fragile** (safety guards absent), **misaligned** (deployed ≠ HEAD; spec §4.3/§4.4 unmet), partly **underbuilt** (no auth, no arg validation). --- ## main findings ### 1. Every workspace capability is registered disabled — the product's only working provider cannot execute - **problem:** `BuildCapabilities()` never sets `Enabled`, so it defaults to `false`. - **evidence:** `internal/provider/workspace_mcp.go:235-249` — the `domain.Capability` literal omits both `Enabled` and `TimeoutSeconds`, leaving them zero-valued. Note that the same function *skips* allowlist-disabled tools at `:226`, so it plainly intends everything it emits to be live: this is an omitted field, not a policy decision. A throwaway test at HEAD printed `Enabled=false RequiresConfirmation=false TimeoutSeconds=0`. `internal/execution/engine.go:41` then returns `ErrCapabilityDisabled` → 403 for all 19 capabilities. - **impact:** Maven's voice execute path can never succeed against a fresh database. Nothing in the suite catches it — no test executes a workspace-provided capability end to end. - **action:** **repair.** Set `Enabled` from risk tier and `TimeoutSeconds` explicitly. Must not land alone — see finding 2. ### 2. Fixing (1) naively arms unconfirmed docker stop/restart on a publicly-proxied, unauthenticated port - **problem:** `BuildCapabilities` also leaves `RequiresConfirmation=false` for `risk: medium` docker start/stop/restart, and the HTTP API has no authentication whatsoever. - **evidence:** `Maven/deploy/ecosystem/nginx.conf:32-42` proxies `hexis.kvmx.ru:80` → `127.0.0.1:9741` with no auth block; `internal/api/handler.go:37` exposes `POST /api/v1/execute`; `handler.go:115-166` lets any caller `POST /api/v1/capabilities` with `provider`+`operation` of their choosing. Precisely: `handler.go:142` derives the correct spec §4.3 default (`enabled := req.Risk != "destructive"`), and then `:143-145` lets an optional `enabled` field in the request body override it. The guard is implemented and then made opt-out by the untrusted caller — worse than simply absent, because it reads as enforced. `requires_confirmation` (`:159`) is copied from the body with no derivation at all. - **concrete attack path:** `POST http://hexis.kvmx.ru/api/v1/capabilities {"name":"x","provider":"workspace_mcp","operation":"ssh.exec","enabled":true}` then `POST /api/v1/execute`. The allowlist blocks that specific one (`workspace_mcp.go:129-135` re-checks `ssh.exec` is disabled) — but `docker.stop_container` is allowed, and self-registration bypasses `requires_confirmation` entirely. - **impact:** remote unauthenticated container control. - **action:** **security, blocking.** Require a shared token on `/api/v1/`; derive `requires_confirmation` from risk tier server-side; and stop honouring `enabled` from the request body entirely rather than treating it as an override. ### 3. Hexis accepts free-text targets — the one thing the spec says it must never do - **problem:** `target_entity_id` is accepted as any non-empty string. Nothing checks the `ent_` shape, existence in Nexus, or `capability.TargetTypes`. - **evidence:** `handler.go:213-216` checks non-empty only; `engine.go:44-46` only compares against a *pinned* `TargetEntityID`, which is `""` for all 19 registered capabilities. Spec §4.3: "Hexis never accepts a free-text target. Ever." - **impact:** the invariant "Maven must not invent mappings" is unenforced at the point it matters. - **action:** **repair.** Validate the target against Nexus `GET /api/v1/entities/{id}` (that endpoint exists) plus a `TargetTypes` match, before creating the execution. - **note:** the spec's stronger *blessing* guard (§4.3) is **not implementable today** — `grep -rni bless` across `nexus/` returns nothing. Nexus has no blessing concept. That's a cross-service gap, not a Hexis bug. ### 4. Audit fields are written but never read back, or never written at all - **problem:** `confirmation_id` is INSERTed but absent from every SELECT, so it always reads back empty. `causation_id` isn't in the `executions` schema at all, yet `engine.go:79` sets it on the struct — the field is populated in memory with nowhere to land. - **evidence:** `confirmation_id` appears in the INSERT at `sqlite.go:295` and in **none** of the three executions SELECTs (`:313`, `:369`, `:418`). `causation_id` exists only on `hexis_events` (`:119`, `:502`), never on `executions`. Also missing vs spec §4.1: `capability_version` on executions (it exists on `confirmations` at `:134`, so the precedent is there). - **impact:** "which confirmation authorized this destructive act" is unanswerable from the API — the core audit question. Silent, not a crash. - **action:** **repair.** Add the columns as a migration, persist and select them. ### 5. `contract_test.go` asserts the opposite of current behavior and passes anyway - **problem:** three tests are prose, not verification. `TestContract_ChangesSinceAlwaysZero` documents a bug that commit `74b19e0` already fixed, and can only `t.Logf`. `TestContract_TimeoutGoroutineNotCancelled` is a bare `t.Log`. `TestContract_DestructiveCapabilityDisabledByDefault` re-implements `risk != "destructive"` inside the test and asserts on its own local variable — it would pass if the handler were deleted. - **evidence:** `internal/execution/contract_test.go:212-299`. - **impact:** the suite's green status overstates coverage; a misnamed test actively misleads about a fixed bug. - **action:** **cleanup.** Delete the three; the real coverage gap is an end-to-end execute over a provider-registered capability. ### 6. Read-only failures are reported as `unknown`, and timed-out provider calls leak - **problem:** `runWithTimeout` (`engine.go:161-186`) can't cancel — `Provider.Execute` takes no `context`. The goroutine runs to the HTTP client's 60s timeout, outliving the 30s capability timeout. - **evidence:** a POST of a `read`/`read_only` capability against the live service while workspace-mcp was down (`:9930` → 503) hung past 5s. Per spec §4.3 that lands `outcome=unknown` — "may or may not have completed the side effect" — for a call that by definition has none, and `unknown` is never retried. - **impact:** read failures are unretryable and indistinguishable from genuinely ambiguous mutations. - **action:** **refactor.** Add `ctx` to the `Provider` interface (2 implementations), and let `read_only=true` timeouts resolve to `failed`. ### 7. Race: the one-in-flight-per-(capability,target) guard is check-then-insert - **problem:** `engine.go:56-58` queries, then `engine.go:92` inserts, with no uniqueness constraint between them. - **evidence:** no partial unique index on `(capability_id, target_entity_id)` where `status='started'`; the existing `idx_executions_inflight` is non-unique (`sqlite.go:561`). - **impact:** two concurrent requests both proceed. Narrow window, single writer, low likelihood — but the spec states it as a hard guard and the existing test only covers the sequential case. - **action:** **repair**, via a partial unique index. ### 8. Deployed binary is older than HEAD; registered capabilities never refresh - **problem:** `cmd/hexisd/main.go:80-83` skips any capability whose ID already exists, so allowlist edits to risk/target_type/enabled never propagate to an existing database. - **evidence:** live GET returns no `enabled` field → pre-`89d8433` build. Rows dated 2026-07-19. - **impact:** config drift; even the correct fix for (1) won't reach the running instance's existing rows. - **action:** **repair.** Reconcile on startup (update when the derived record differs), and redeploy. --- ## keep Confirmation lifecycle and its tests; the storage layer's explicitness; `timeout ⇒ unknown`; the provider registry (2 real implementations justify it); the allowlist-as-config design; `nexusclient`; pure-Go sqlite + distroless build. ## remove - `internal/provider/systemd.go` — no `systemctl` in the distroless image and no capability ever registers it; it's the spec's §4.2 `systemd_dbus` slot filled with a shell-adjacent stub. Delete, or replace with D-Bus when a real need appears. - `Server.ListenUnix` (dead; never called). - `schema_migrations` table (unused; `PRAGMA user_version` is the real mechanism). **Caveat:** confirm no already-deployed database depends on it before dropping — finding 8 establishes the live instance is several commits behind, so its schema state is not HEAD's. - `execution.ResolveRequest` / `ResolveResult` + `MarshalJSON` (superseded by `nexusclient`). - `Engine.emitEvent`, `Registry.List`, `DiscoveredTools`, `Capability.IsDestructive`, `ifString`. - `EventCapabilityUnavailable`, `EventExecutionDenied`, `ExecutionDenied`, and 8 unused `Err*` values. - The three vacuous contract tests. ## repair now 1. Auth on `/api/v1/` + server-derived `requires_confirmation` (finding 2) — **before anything else**. 2. `Enabled` / `TimeoutSeconds` / `RequiresConfirmation` in `BuildCapabilities`, plus startup reconciliation (1, 8). 3. Target validation against Nexus + `TargetTypes` (3). 4. Persist/read `confirmation_id`, `causation_id`, `capability_version` (4). 5. Partial unique index for in-flight (7). 6. Delete the dead code and misleading tests (5, remove). ## refactor later - `context` through `Provider.Execute` (6). - Collapse the four competing wire shapes for a capability — `handler.go:96-108` (which emits both `id` and `capability_id`), `adapter.go:147-157`, `pkg/client`, and Maven's vendored copy — into one serializer. - `GET /api/v1/executions?entity_id=&since=` (spec §4.5, Command Center needs it). - A `Makefile` and a README: there is no onboarding doc at all. ## rewrite only if Nexus gains blessing and versioned capabilities `(capability_id, version)` as the primary key, *and* multi-writer access becomes real. Neither is true; incremental repair is clearly cheaper. The confirmation logic is the hidden knowledge here and it is correct — do not throw it away. ## verification plan - `go build ./... && go vet ./... && go test ./...` — all currently pass. `gofmt -l` flags `internal/provider/systemd.go`, `internal/provider/workspace_mcp.go`, `pkg/client/client.go` as unformatted at HEAD. - New tests, each pinning a finding: - execute a `BuildCapabilities`-registered capability end-to-end (1) - unauthenticated `/api/v1/execute` rejected (2) - non-`ent_` / unknown target rejected (3) - round-trip `confirmation_id` through `GET /executions/{id}` (4) - concurrent duplicate execute → exactly one row (7) - Live: rebuild, redeploy, `curl` a read capability once workspace-mcp is back (it is 503 as of this review), confirm `succeeded` and non-empty `changes`. ## uncertainties - **Unknown:** whether `hexis.kvmx.ru:80` is actually resolvable from outside — the nginx config was read, external reachability was not tested. The severity of finding 2 depends on this; the missing-auth defect stands either way. - **Unknown:** whether blessing is planned for Nexus soon, or whether the target-existence check in finding 3 is the intended permanent guard. - **Assumption:** Maven is the only consumer. Based on a repo-wide grep of `/home/kami/apps`; an out-of-tree MCP client would not appear. - **Confirmed but unexplained:** the live instance is ~4 commits behind. Unclear whether that is a stalled deploy or deliberate. --- ## second-reviewer verification (2026-07-30) An independent pass re-checked the load-bearing claims against the code at HEAD rather than accepting them. **The verdict and all eight findings stand.** Confirmed directly: - **Finding 1** — `workspace_mcp.go:235-249` omits `Enabled` and `TimeoutSeconds`; `engine.go:42` rejects on `!capability.Enabled`. The execution path is dead as described. - **Finding 2** — no `auth`, `token`, or `Authorization` handling exists anywhere under `internal/api/`. The `hexis.kvmx.ru:80` server block proxies straight to `127.0.0.1:9741` with only `X-Forwarded-*` headers set and no auth directive. - **Finding 4** — verified by comparing the INSERT column list against all three SELECT column lists; `confirmation_id` is write-only. `causation_id` is absent from the `executions` DDL. - **Finding 5** — the three tests are as characterised. `TestContract_DestructiveCapabilityDisabledByDefault` computes `enabled := risk != "destructive"` as a local variable and asserts on that, touching no production code. `TestContract_TimeoutGoroutineNotCancelled` is a single `t.Log` with no assertions. - **Finding 7** — `sqlite.go:143` creates `idx_executions_inflight` as a plain `CREATE INDEX`, not `CREATE UNIQUE INDEX`, and there is no partial unique index anywhere. - **Finding 8** — `cmd/hexisd/main.go:76-79` `continue`s on any capability whose ID already resolves, so derived changes never reconcile. - **Verification plan** — `gofmt -l` flags exactly the three files named. Amendments made above, none of which change a conclusion: finding 2's description of `enabled` was corrected (it is a caller-supplied override of a correct server default, not "forced true"), finding 4's line references were corrected to the actual INSERT/SELECT sites, and the `schema_migrations` removal gained a deployed-schema caveat. One emphasis difference: finding 4 reads as more serious than "silent, not a crash" suggests. The confirmation lifecycle is the part of this codebase the review rightly calls correct and worth keeping — but with `confirmation_id` unreadable, that correctness cannot be demonstrated after the fact from the API. A guard you cannot audit is hard to trust under incident review, which raises finding 4 close to the priority of the target-validation work in finding 3. Agreement on sequencing is unqualified; see below. ## note on sequencing Findings 1 and 2 are coupled. Fixing the dead-capability bug without adding auth and confirmation defaults would turn a broken execution path into a remotely reachable one. They should land together as one coherent change.