plans: review and archive all plans to done/
Architecture has changed drastically (hexagonal refactor, web client extraction). Every active plan has been reviewed, annotated with 'Completed' or 'Won't do' status, and moved to plans/done/. Completed (7): gaps-and-improvements, liveness-drift, gated-execution, nomos-code-review, codebase-cleanup, mascot-physics, backend-eval Won't do (7): prometheus-lxc, control-room-webui, activity-gaps, activity-timeline, frontend-os-apps, haos-capability-gaps, arr-audit
This commit is contained in:
73
plans/done/2026-07-05-oikos-prometheus-lxc.md
Normal file
73
plans/done/2026-07-05-oikos-prometheus-lxc.md
Normal file
@@ -0,0 +1,73 @@
|
||||
# Oikos metrics stack — Prometheus LXC (planned)
|
||||
|
||||
**Reviewed 2026-08-16 — Status: Won't do** — planned but never started; file paths and architecture references are stale post-hexagonal refactor.
|
||||
|
||||
Lifecycle state: **planned** (see [seeds/ontology.yaml](../seeds/ontology.yaml)
|
||||
lifecycle). No LXC exists yet — this is the plan doc that state requires
|
||||
before provisioning starts. Do not add a host entry with a guessed
|
||||
`pve_id` until the LXC is actually created; Proxmox assigns the real ID
|
||||
at `pct create` time.
|
||||
|
||||
## Why
|
||||
|
||||
Week-3 reliability layer (see [OIKOS.md](../.agents/OIKOS.md)) wants trend
|
||||
signals — "disk full in ~9 days at current rate", temperature creep —
|
||||
which need a real time-series store. The Go scheduler
|
||||
([internal/scheduler/scheduler.go](../internal/scheduler/scheduler.go))
|
||||
currently does point-in-time threshold checks only; Prometheus is the one
|
||||
new piece of infrastructure the 30-day roadmap calls for.
|
||||
|
||||
## Note: LXC 131 is teddycloud
|
||||
|
||||
The drift detector (`drift` check kind in `check_defs`) found
|
||||
`pve_id 131` live on hubris — this is **teddycloud** (`lxc:teddycloud`),
|
||||
which is now documented in `seeds/knowledge.yaml`. The pve_id range is
|
||||
fully accounted for (101-134). Next available VMIDs start at 135.
|
||||
|
||||
## Plan
|
||||
|
||||
- **Host:** hubris (per the earlier decision: new LXC, not co-located).
|
||||
- **Role:** `metrics` (Prometheus + local TSDB retention; no Grafana yet —
|
||||
the Week-4 Oikos Console renders its own sparklines from the Prometheus
|
||||
HTTP API, per the plan's Week-3 scope decision).
|
||||
- **Networking:** LAN + mesh-gated only, no public ingress (matches
|
||||
`mcp` / `secrets-issuance` `MESH_SUBNETS` pattern) — Prometheus
|
||||
exposes host/service metadata that shouldn't be public.
|
||||
- **Scrape targets:** node_exporter on hubris and strong (Proxmox hosts)
|
||||
+ any LXC the scheduler needs disk/temp trend data from beyond what
|
||||
`pct`/`df` already gives (start with hubris + strong only; expand only
|
||||
if a specific signal needs it).
|
||||
- **Storage:** local LXC rootfs is sufficient (metrics-only workload,
|
||||
short retention — no `/mnt/library` mount needed).
|
||||
|
||||
## Provisioning steps (once pve_id is assigned)
|
||||
|
||||
1. Add entity to `seeds/inventory.yaml` with state `planned` and the
|
||||
assigned pve_id. Run `oikos seed` to ingest.
|
||||
2. `pct create <new-id> ...` on hubris — next available PVE ID is 135.
|
||||
3. Install Prometheus + node_exporter (Debian package or binary release —
|
||||
decide at implementation time; no strong preference recorded here).
|
||||
4. Point node_exporter at hubris + strong (either install locally on each,
|
||||
or scrape via SSH-tunneled metrics — install locally is simpler).
|
||||
5. Transition to `active` via `PATCH /api/v1/entities/{slug}` or
|
||||
operator approval through the execution flow.
|
||||
6. Add a `prometheus` check kind to `check_defs` in the scheduler
|
||||
(`internal/scheduler/scheduler.go`) so trend signals can query
|
||||
Prometheus `rate()` alongside the current SSH `df` probe, giving
|
||||
"full in ~9 days" predictions instead of only point-in-time
|
||||
percentages. Register a `prometheus` check def via `POST /api/v1/checks`.
|
||||
7. Run `oikos export` to regenerate `seeds/inventory.yaml` for git.
|
||||
|
||||
## Open question for the operator
|
||||
|
||||
Package choice (apt `prometheus` vs upstream binary release) and exact
|
||||
scrape interval aren't decided here — pick at implementation time based
|
||||
on what's easiest to keep patched via MCP `request_execution(action="apt_upgrade")`.
|
||||
|
||||
## Changelog
|
||||
|
||||
### 2026-07-08 — Go references updated
|
||||
Replaced Python references (`oikos/scheduler.py`, `oikos/drift.py`,
|
||||
`bin/homelab`, `homelab client add`) with Go equivalents: `internal/scheduler/`,
|
||||
`check_defs` drift kind, MCP `request_execution`, `seeds/inventory.yaml`.
|
||||
LXC 131 identified as teddycloud (PVE range 101-134 fully accounted for).
|
||||
231
plans/done/2026-07-08-control-room-webui.md
Normal file
231
plans/done/2026-07-08-control-room-webui.md
Normal file
@@ -0,0 +1,231 @@
|
||||
# 2026-07-08 — Control room web UI
|
||||
|
||||
**Reviewed 2026-08-16 — Status: Won't do** — embed architecture removed by Wails client/server split; UI work moved to dtoro/oikos-web.
|
||||
|
||||
**Status:** In Progress (audited 2026-07-11 — still accurate; remaining gaps:
|
||||
`signal.acked`/`signal.resolved`/`signal.muted` and `relationship.created`/
|
||||
`relationship.ended` API calls don't emit `observability.Event`, and
|
||||
trusted-proxy header auth for Authentik was never added to `combinedAuth`).
|
||||
**Superseded (2026-07-12):** the embed architecture below (`go:embed
|
||||
all:web/dist`, served at `/ui/`) was removed —
|
||||
[2026-07-12-wails-desktop-app.md](2026-07-12-wails-desktop-app.md) Phase 0
|
||||
separates the SPA from the `oikos` binary into a standalone static build,
|
||||
served at `/` (no `/ui/` prefix), talking to the API over bearer-token
|
||||
auth (the dev-open bypass mentioned nowhere in this plan was also removed).
|
||||
The trusted-proxy-header gap noted above is moot under the new model — every
|
||||
route requires a real bearer token regardless of what's in front of it. M1-M3
|
||||
and the SPA/component work below are unaffected; only the packaging and auth
|
||||
sections are stale.
|
||||
N0-N3 (Nomos amendment: chat home + sessions), M1
|
||||
(dashboard/summary, Overview, Entities table, live event feed, shadcn-svelte
|
||||
component system), M2 (Operations ledger with approve/deny + cancel, Signals
|
||||
page with ack/resolve/mute, live nav badges), and M3 (graph explorer with
|
||||
`include=status` health coloring, per-relationship-type edge coloring +
|
||||
legend/filter, node-type filter, node search/highlight, entity detail page
|
||||
with uPlot metric charts, `#/entity/:slug` route) complete 2026-07-08. Event
|
||||
gap-fill (M2's
|
||||
other half) landed earlier in commit e8e230b, and approval creation's FK bug
|
||||
(gaps-plan A1) was already fixed, unblocking M2. While building M3, also
|
||||
fixed `GET /metrics` to make the `metric` query param genuinely optional
|
||||
(server now reports every metric recorded for the entity in range) — the
|
||||
implementation previously 400'd when it was omitted, contradicting its own
|
||||
documented-optional spec. M4 (agent activity, knowledge search page, audit,
|
||||
correlation grouping, polish) remains.
|
||||
|
||||
## Goal
|
||||
|
||||
A realtime "control room" web UI for inspecting the state of Oikos and all its
|
||||
entities — graphs, tables, and lists — that updates live as an agent (Hermes)
|
||||
interacts with the system: executions appearing, approvals firing, health
|
||||
changing, the entity graph mutating. Inspect **and act**: approve/deny pending
|
||||
approvals and ack/resolve/mute signals directly from the UI.
|
||||
|
||||
---
|
||||
|
||||
## Stack (decided)
|
||||
|
||||
**Svelte 5 + Vite + TypeScript SPA**, compiled to static assets in
|
||||
`web/dist`, embedded into the existing `oikos` binary via
|
||||
`go:embed all:web/dist` in a new `internal/httpapi/ui.go`, served at `/ui/`
|
||||
(redirect `/` → `/ui/`, SPA fallback to `index.html`).
|
||||
|
||||
Why: the hard requirements (force-directed graph, time-series charts, one SSE
|
||||
stream patching many widgets) are client-side-JS problems; Svelte's reactive
|
||||
stores map 1:1 onto "SSE event mutates shared state, every widget reacts";
|
||||
the compiled runtime keeps the embed small; and `openapi-typescript` generates
|
||||
frontend types from `api/openapi.yaml` — the frontend twin of the repo's
|
||||
oapi-codegen contract-first discipline (ADR-0004). Embedding preserves the
|
||||
single-binary story: no new container, no CORS, no Caddy changes.
|
||||
|
||||
Dependencies kept minimal:
|
||||
- `d3-force` — graph physics only; render SVG/canvas by hand
|
||||
- `uPlot` — ~45 KB canvas time-series, ideal for `/metrics` rollups
|
||||
- `openapi-typescript` — dev-only, generates `api-types.d.ts`
|
||||
- No SvelteKit (no SSR wanted — the Go binary is the server); hash router.
|
||||
|
||||
*Amendment 2026-07-08 (M1):* component library is
|
||||
[shadcn-svelte](https://www.shadcn-svelte.com/) over Tailwind CSS v4
|
||||
(`@tailwindcss/vite`), not hand-rolled CSS — Table, Card, Badge, Sidebar,
|
||||
Sheet, Select, Input, Button, Tabs, ScrollArea, Tooltip, Dialog,
|
||||
Dropdown-menu, Sonner installed via `npx shadcn-svelte add`. The existing
|
||||
dark GitHub-style palette (`app.css`) was ported into shadcn's CSS-variable
|
||||
theme contract (`--background`, `--card`, `--primary`, etc. under
|
||||
`@theme inline`) so old and new components share one palette. `d3-force` /
|
||||
`uPlot` remain the plan for the graph/charts milestones (M3), unaffected by
|
||||
this change.
|
||||
|
||||
Build integration: commit a placeholder `web/dist/index.html` so backend-only
|
||||
`go build` never breaks; `make ui` runs the Vite build; add a node stage to
|
||||
`compose/oikos/Dockerfile` (3-stage: node → go → runtime). Local dev:
|
||||
`vite dev` proxying `/api` → `:8090`.
|
||||
|
||||
---
|
||||
|
||||
## Realtime
|
||||
|
||||
Reuse the existing pipeline: migration 008's `pg_notify('oikos_events')`
|
||||
trigger → `internal/httpapi/sse.go` broker → `GET /api/v1/events/stream`
|
||||
(Last-Event-ID replay, 15s heartbeat). One shared `EventSource` in a Svelte
|
||||
store; pages subscribe by event type and either patch state from `event.data`
|
||||
or trigger a targeted refetch. Scheduler and actuator are separate processes
|
||||
but share Postgres, so their events reach the api role's LISTEN automatically.
|
||||
|
||||
### Event emission gap-fill (required)
|
||||
|
||||
Today only 5 event types are emitted (`entity.created`, `entity.updated`,
|
||||
`client.enrolled`, `entity.provisioned`, `execution.requested`). Most of what
|
||||
the control room must show live is silent. Add `observability.Event(...)`
|
||||
calls at:
|
||||
|
||||
| Event | Site |
|
||||
|-------|------|
|
||||
| `signal.raised` / `signal.resolved` | `internal/scheduler/scheduler.go` (~115 / ~145) |
|
||||
| `health.changed` (on transition only, to avoid flooding) | scheduler `entity_status` writes (~104/135/156) |
|
||||
| `signal.acked` / `signal.muted` / `signal.resolved` (API side) | `internal/httpapi/impl.go:545/582/619` |
|
||||
| `approval.created` | `internal/mcp/server.go` `createApproval` — depends on bug A1 in [gaps plan](2026-07-08-oikos-gaps-and-improvements.md) |
|
||||
| `approval.decided` | `internal/httpapi/phase3.go:871` |
|
||||
| `execution.started/completed/failed/cancelled` | `internal/actuator/actuator.go`, `phase3.go:746` |
|
||||
| `relationship.created` / `relationship.ended` | `phase3.go:1560/1621` |
|
||||
|
||||
Agent activity: poll `GET /agent-activity` every ~5s rather than duplicating
|
||||
every tool call into `events`.
|
||||
|
||||
---
|
||||
|
||||
## API surface
|
||||
|
||||
Existing endpoints already cover nearly everything the UI needs:
|
||||
`/graph?root=&depth=&rel_type=` (impl.go:268), `/health` fleet rollup
|
||||
(impl.go:481), `/metrics?rollup=raw|1h|1d|auto&from=&to=`, `/trends/{id}`,
|
||||
`/events` (filterable) + `/events/stream` (SSE), `/signals` + ack/resolve/
|
||||
mute, `/executions`, `/approvals` + `POST /approvals/{id}/decision`,
|
||||
`/agent-activity`, `/knowledge/search`, `/audit`, entities CRUD, `/ontology`.
|
||||
|
||||
New endpoints (spec-first in `api/openapi.yaml`, regen, then implement):
|
||||
|
||||
1. `GET /api/v1/dashboard/summary` (new `internal/httpapi/dashboard.go`) —
|
||||
one round-trip for the overview: entity counts by type/state, health
|
||||
rollup from `entity_status`, open signals by severity, pending approvals,
|
||||
executions by state (24h), event-rate sparkline (count/5min from the
|
||||
`events` hypertable).
|
||||
2. `GET /graph?include=status` — join `entity_status` so graph nodes can be
|
||||
colored by health (preferred over a separate bulk-status endpoint).
|
||||
|
||||
---
|
||||
|
||||
## Auth & deployment
|
||||
|
||||
- Serve `/ui/*` assets without `combinedAuth` — Caddy/Authentik already gates
|
||||
the `oikos.hubris.network` vhost, and the assets are public JS/CSS.
|
||||
- **Trusted-proxy header auth** for API calls from the browser: extend
|
||||
`combinedAuth` (`internal/httpapi/server.go:157`) with a config flag
|
||||
(`OIKOS_TRUSTED_PROXY_AUTH`) — when no Bearer token is present but Authentik
|
||||
forward-auth headers (`X-Authentik-Username`/`-Email`) are, resolve an
|
||||
operator actor. Caddy must inject these via `forward_auth` and strip inbound
|
||||
`X-Authentik-*` from clients. Result: the UI needs no token handling, and —
|
||||
critically — `EventSource` works unmodified (it cannot set Authorization
|
||||
headers).
|
||||
- Docker: node build stage in `compose/oikos/Dockerfile`; no compose or Caddy
|
||||
routing changes.
|
||||
|
||||
---
|
||||
|
||||
## Pages
|
||||
|
||||
Persistent nav + live top status strip (health dots, open-signal badge,
|
||||
pending-approval badge):
|
||||
|
||||
0. **Agent chat (home view)** — *amendment 2026-07-08, see the
|
||||
[Nomos resident agent plan](2026-07-08-nomos-resident-agent.md)*: the
|
||||
default view at `/ui/#/` is a conversation with the resident Nomos agent
|
||||
(formerly Hermes; streamed via `/agent/chat`), with a live right rail
|
||||
showing pending approvals (decidable in place), recent events, and health.
|
||||
A persistent chat drawer is reachable from every other page. Lands with
|
||||
resident-agent milestone N3; until then, Overview is the home view.
|
||||
1. **Overview** — summary cards from `/dashboard/summary`, event-rate
|
||||
sparkline, live event ticker, top degraded entities.
|
||||
2. **Graph explorer** — d3-force over `/graph`; node color = health; filters
|
||||
by entity type / rel_type / root+depth; click → side panel with attributes
|
||||
+ blast radius; live mutation from `entity.*` / `relationship.*` /
|
||||
`health.changed` events.
|
||||
3. **Entity detail** — attributes, relations mini-graph, blast radius, uPlot
|
||||
charts (`/metrics`, `/trends`), entity-scoped events, knowledge, open
|
||||
signals, executions.
|
||||
4. **Entities table** — filter/sort by type/state/health, live badges.
|
||||
5. **Operations ledger** — executions + approvals panes with **approve/deny
|
||||
buttons** (`POST /approvals/{id}/decision`); live via `execution.*` /
|
||||
`approval.*` events; grouped by `correlation_id`. The "watch Hermes work"
|
||||
page.
|
||||
6. **Signals** — tabs by state, severity filter, **ack/resolve/mute** actions.
|
||||
7. **Live event feed** — full stream tail with filters, pause,
|
||||
correlation-id clustering (one agent action = one cluster), scroll-back via
|
||||
`GET /events`.
|
||||
8. **Agent activity** — polled `/agent-activity` timeline joined to
|
||||
executions/approvals by correlation_id.
|
||||
9. **Knowledge** — FTS search over `/knowledge/search`. **Audit** — low
|
||||
priority.
|
||||
|
||||
---
|
||||
|
||||
## Milestones
|
||||
|
||||
- **M1 (MVP):** scaffold + embed + proxy auth; Overview, Entities table, Live
|
||||
event feed over SSE. Proves the pipeline end-to-end (agent creates an
|
||||
entity → it appears live in the browser).
|
||||
- **M2:** event gap-fill (all sites above); Operations ledger with live
|
||||
approvals + decision buttons; Signals page. The payoff milestone. *Depends
|
||||
on gaps-plan bug A1 for approvals to exist at all.*
|
||||
- **M3:** graph explorer with live mutation; entity detail with charts;
|
||||
`/dashboard/summary`; `include=status`.
|
||||
- **M4:** agent activity, knowledge, audit, correlation grouping, polish.
|
||||
|
||||
## File layout
|
||||
|
||||
```
|
||||
web/
|
||||
package.json vite.config.ts
|
||||
src/
|
||||
main.ts App.svelte router.ts
|
||||
lib/api.ts lib/api-types.d.ts
|
||||
lib/stores/{events.ts, summary.ts}
|
||||
pages/{Overview,Graph,Entity,Entities,Ops,Signals,Events,Agent,Knowledge}.svelte
|
||||
dist/index.html # committed placeholder
|
||||
internal/httpapi/ui.go # go:embed + SPA fallback at /ui
|
||||
internal/httpapi/dashboard.go # GET /dashboard/summary
|
||||
internal/httpapi/server.go # mount /ui; trusted-proxy auth
|
||||
api/openapi.yaml # new paths
|
||||
internal/{scheduler/scheduler.go, mcp/server.go, actuator/actuator.go,
|
||||
httpapi/{impl,phase3}.go} # missing event emissions
|
||||
Makefile compose/oikos/Dockerfile # ui build targets, node stage
|
||||
```
|
||||
|
||||
## Verification
|
||||
|
||||
- M1: run `oikos api` locally, open `/ui/`, confirm Overview + Entities render
|
||||
from live API; `POST /api/v1/entities` from curl and watch it appear in the
|
||||
event feed without refresh.
|
||||
- M2: drive `request_execution` through Hermes and watch the approval appear
|
||||
on the Operations ledger, decide it from the UI, and see the execution state
|
||||
advance live.
|
||||
- Backend-only `go build ./...` succeeds without a node toolchain (placeholder
|
||||
dist).
|
||||
257
plans/done/2026-07-08-liveness-drift-and-ux-cohesion.md
Normal file
257
plans/done/2026-07-08-liveness-drift-and-ux-cohesion.md
Normal file
@@ -0,0 +1,257 @@
|
||||
# 2026-07-08 — Liveness, drift, and UX cohesion
|
||||
|
||||
**Reviewed 2026-08-16 — Status: Completed** — Phases 1–4 implemented and deployed; Phase 5 deferred and superseded by web client extraction.
|
||||
|
||||
**Status:** In Progress — Phases 1–4 code complete and now deployed
|
||||
(re-verified 2026-07-12: mac-mini was redeployed from `main` that day for
|
||||
unrelated auth work — plans/2026-07-12-wails-desktop-app.md — which carried
|
||||
every commit up to that point, including this plan's, so "not yet deployed"
|
||||
below is stale). Phase 5 deferred. (Audited 2026-07-11 — still accurate;
|
||||
prompt caching within Phase 4 also confirmed not implemented.)
|
||||
|
||||
- **Phase 1 (drift/staleness):** done. Health/metrics/events misattribution
|
||||
fix, staleness sweep, `/entities` health+freshness, dashboard/fleet-health
|
||||
`check`-entity exclusion, Entities table health column. Migration 016
|
||||
applied to the live dev DB (safe cleanup, additive-only).
|
||||
- **Phase 2 (detail sidebar + legibility):** done. `EntityDetailContent`
|
||||
extracted and shared between the full `#/entity/:slug` page and a new
|
||||
`EntitySheet` opened from the Entities table (master-detail, no
|
||||
navigation). Adds a **Monitoring** card (per-entity `check_defs`: kind,
|
||||
interval, enabled/disabled with click-to-toggle) and renders attributes as
|
||||
a key/value list instead of raw JSON.
|
||||
- **Phase 3 (sessions rejoin chat):** done. Fixed the click-does-nothing bug,
|
||||
added a session rail inside Chat, fixed the local dev proxy to match
|
||||
production's `/agent` prefix-stripping. **Also found and fixed a real
|
||||
latent bug**: persisted `tool_calls` store the `tool_use`/`tool_result` as
|
||||
two entries sharing one `id`; Chat.svelte's keyed `{#each tool (tool.id)}`
|
||||
threw on the duplicate key and silently blanked the entire message list.
|
||||
This had presumably never been noticed because sessions were never
|
||||
clickable before this fix. Fixed in `chat.ts` by merging tool_calls by id
|
||||
before rendering.
|
||||
- **Phase 4 (agent efficiency):** core piece done — prior turns' tool
|
||||
calls/results are now replayed into the conversation (previously dropped
|
||||
entirely), and a compact live fleet-health snapshot is injected into the
|
||||
system prompt each turn so the agent starts oriented. Prompt caching is
|
||||
**not done** (lower priority, no measured regression without it).
|
||||
Reconsidering the default model — done, but not by this plan: switched to
|
||||
`deepseek/deepseek-v4-pro` on 2026-07-10 (`cmd/nomos/agent.go:70`) for
|
||||
reliability, per that commit's own comment ("the flash tier over-narrates,
|
||||
occasionally emits canned refusals, and is unreliable at multi-step tool
|
||||
use").
|
||||
- **Phase 5 (CRUD):** `PatchEntity` and a full `/checks` CRUD API
|
||||
(list/create/patch, including enable/disable) already existed server-side;
|
||||
the new Monitoring card's toggle uses `PatchCheck`. **Not done**: a
|
||||
"run check now" endpoint (no scheduler on-demand entrypoint exists yet),
|
||||
relationship editing, and an entity attribute editor UI.
|
||||
|
||||
Addresses five felt problems with the current system: (1) the agent reports
|
||||
stale machine state as if it were fresh, (2) sessions can't be opened and feel
|
||||
disconnected from chat, (3) the Nomos agent re-derives state every turn and
|
||||
wastes iterations, (4) the UI feels dead — tables with no context, no sense of
|
||||
what is monitored, (5) no way to inspect or customize entities and their checks.
|
||||
|
||||
The unifying UX principle for this plan: **master-detail with a detail
|
||||
sidebar**, not full-page navigation. Selecting an entity, session, or signal
|
||||
opens a right-hand detail panel over the current list, so the operator keeps
|
||||
context and drills in without losing their place. Full pages remain
|
||||
addressable (deep links) but are no longer the primary way to inspect a row.
|
||||
|
||||
Sequencing is driven by pain: **drift/staleness is Phase 1.**
|
||||
|
||||
---
|
||||
|
||||
## Root causes (verified in code)
|
||||
|
||||
### Drift / staleness — root cause was worse than a missing TTL
|
||||
Live-DB inspection (`oikos-postgres-1`) found the real cause: `check_defs` has
|
||||
two entity references — `entity_id` (the internal probe/"check" entity) and
|
||||
`target_id` (the host/service actually being observed). The scheduler wrote
|
||||
`entity_status`, `metric_samples`, and scheduler-sourced `events` keyed by
|
||||
`cd.EntityID` (the probe) instead of `cd.TargetID` (the target) —
|
||||
[scheduler.go:110-171](../internal/scheduler/scheduler.go) (pre-fix). Verified
|
||||
against the live database:
|
||||
|
||||
```
|
||||
entity_status by type: only type='check' rows ever had real health (24
|
||||
healthy, 1 down); every host/service/lxc/vm/proxmox-host was frozen at
|
||||
'unknown' since creation.
|
||||
metric_samples: 17,559 rows, 100% attached to type='check' entities — zero
|
||||
attached to any real host or service.
|
||||
events: 45 of 46 scheduler-sourced rows attached to type='check' entities.
|
||||
```
|
||||
|
||||
So this wasn't staleness in the TTL sense — the entities you actually care
|
||||
about (`host:hubris`, `service:authentik`, etc.) **never received an
|
||||
observation at all**. Every health check, metric, and event the scheduler
|
||||
produced was filed under an internal bookkeeping entity the UI doesn't even
|
||||
surface distinctly. This is the literal mechanism behind "the agent tells me
|
||||
stale/wrong state."
|
||||
|
||||
**Fixed** (this session): `runCheck`/`resolveSignal` now resolve
|
||||
`targetID := cd.TargetID` and write status/metrics/events there, falling back
|
||||
to the check's own id only if `target_id` is unset. Signals remain keyed by
|
||||
the check entity (unchanged, matches their existing resolution logic). A new
|
||||
migration ([016_fix_check_status_misattribution](../migrations/016_fix_check_status_misattribution.up.sql))
|
||||
deletes the orphaned check-entity `entity_status` rows so rollups stop
|
||||
double-counting probes as monitored entities; historical `metric_samples` on
|
||||
check entities are left as-is (time-series data, not safe to reattribute).
|
||||
|
||||
On top of the misattribution fix, a genuine staleness gap also existed and is
|
||||
now closed: `entity_status.health` was written only when a check ran, with no
|
||||
TTL — a stalled scheduler or disabled check_def would leave the last health
|
||||
value looking current forever.
|
||||
- `last_check_at` is recorded but was never surfaced. The Entities table
|
||||
showed `entity.updated_at` (row mutation time), not observation time
|
||||
([Entities.svelte:108](../web/src/pages/Entities.svelte), pre-fix).
|
||||
- The `/entities` list endpoint returned neither `health` nor `last_check_at`
|
||||
— only `/graph?include=status` and `/fleet/health` did
|
||||
([impl.go:344](../internal/httpapi/impl.go), pre-fix).
|
||||
|
||||
### Sessions
|
||||
- Clicking a session calls `loadSessionMessages()` but never navigates to the
|
||||
chat page ([Sessions.svelte:17](../web/src/pages/Sessions.svelte)); it mutates
|
||||
the chat store while the user stays on the session list, so nothing appears to
|
||||
happen. There is also no session switcher inside Chat.
|
||||
|
||||
### Agent efficiency
|
||||
- Multi-turn history replay **drops all `tool_use`/`tool_result` pairs**; only
|
||||
prior final text is replayed ([agent.go:108-127](../cmd/nomos/agent.go)). Each
|
||||
new turn re-discovers the fleet from scratch, re-calling tools already run.
|
||||
- Cold start: the system prompt injects no fleet snapshot
|
||||
([agent.go:81](../cmd/nomos/agent.go)); default model is
|
||||
`deepseek/deepseek-v4-flash` ([agent.go:34](../cmd/nomos/agent.go)); tool
|
||||
schema + system prompt are rebuilt each call with no prompt caching.
|
||||
|
||||
### Dead UI / no inspection
|
||||
- Entities table = slug/type/name/state/updated; no health, no last-seen, no
|
||||
signal count.
|
||||
- EntityDetail dumps `JSON.stringify(attributes)` raw
|
||||
([EntityDetail.svelte:123](../web/src/pages/EntityDetail.svelte)) and never
|
||||
shows the entity's `check_defs` — the operator cannot see *what is monitored*,
|
||||
when it last ran, or what it returned.
|
||||
- No CRUD anywhere: no entity editor, no check management (enable/disable/edit/
|
||||
run-now), no relationship editing. `check_defs` do not appear in the web app.
|
||||
|
||||
---
|
||||
|
||||
## Phase 1 — Kill the drift (highest priority)
|
||||
|
||||
Goal: the system never presents stale observations as fresh, and freshness is
|
||||
visible everywhere health is.
|
||||
|
||||
**Backend**
|
||||
- Add a staleness sweep to `housekeeping()`
|
||||
([scheduler.go:243](../internal/scheduler/scheduler.go)): for each
|
||||
`entity_status` where `now() - last_check_at > staleAfter` (default
|
||||
`max(3 × check interval, 5m)`), transition health to a new `stale` value and
|
||||
emit a `health.stale` event once (not every pass).
|
||||
- Treat `stale` as a first-class health in dashboard rollups
|
||||
([dashboard.go:57](../internal/httpapi/dashboard.go)) and fleet health
|
||||
([impl.go:516](../internal/httpapi/impl.go)) — do not fold it into `unknown`.
|
||||
- Extend the `/entities` list response with `health` and `last_check_at`
|
||||
(join `entity_status`), so the table can show freshness without N graph calls.
|
||||
- Nomos: when answering about state, tool results should carry `last_check_at`
|
||||
and a stale flag so the agent can hedge ("healthy as of 4m ago") instead of
|
||||
asserting stale data. (Verify the MCP topology/health tools include it.)
|
||||
|
||||
**Frontend**
|
||||
- Entities table: replace the `Updated` column with **health dot + relative
|
||||
"checked 2m ago"**, and add an **open-signal count** badge per row. Stale rows
|
||||
get a distinct muted/amber treatment, not a green dot.
|
||||
- Global header: add an "as of {time}" and make the SSE connection dot a real
|
||||
liveness indicator (last event received, reconnect state).
|
||||
|
||||
**Acceptance:** disable a check or stop the scheduler → within one stale window
|
||||
the affected entity shows `stale` in the table and dashboard, an event fires,
|
||||
and asking Nomos "is X healthy?" yields a freshness-qualified answer.
|
||||
|
||||
---
|
||||
|
||||
## Phase 2 — Detail sidebar + entity legibility (less navigation)
|
||||
|
||||
Goal: inspect any row in place; make an entity's monitoring self-evident.
|
||||
|
||||
- Introduce a reusable **DetailSheet** (right-side panel) used across Entities,
|
||||
Signals, Sessions, Executions. Row click opens the sheet; URL hash updates for
|
||||
deep-linking; Esc / click-away closes. Full `#/entity/:slug` page remains for
|
||||
direct links but reuses the same detail component.
|
||||
- Entity detail content (in the sheet):
|
||||
- Header: slug, type, **health + freshness** ("checked 2m ago" / "stale
|
||||
18m").
|
||||
- **Monitoring card**: the entity's `check_defs` — kind, schedule, enabled,
|
||||
last result + evidence, next run. This is the missing "what is watched."
|
||||
- Attributes rendered as a key/value panel, not raw JSON.
|
||||
- Relations, open signals, recent executions, metrics sparklines (reuse
|
||||
existing EntityDetail sections).
|
||||
- Backend: endpoint to list `check_defs` for an entity with last-result join
|
||||
(currently checks are only visible to the scheduler).
|
||||
|
||||
**Acceptance:** from the Entities list, one click reveals what an entity is,
|
||||
what's monitoring it, when it was last seen, and its open signals — without a
|
||||
full page load or losing the list.
|
||||
|
||||
---
|
||||
|
||||
## Phase 3 — Sessions rejoin chat
|
||||
|
||||
Goal: sessions are openable and live next to the conversation.
|
||||
|
||||
- Fix: clicking a session navigates to `#/chat` and loads it
|
||||
([Sessions.svelte:17](../web/src/pages/Sessions.svelte)).
|
||||
- Add a **session rail inside Chat** (collapsible left list: title, last-active,
|
||||
active highlight) so switching sessions never leaves the chat surface. The
|
||||
standalone Sessions page becomes a thin wrapper / can be retired from nav.
|
||||
- Show session metadata (message count, last actor) and allow rename/delete.
|
||||
|
||||
**Acceptance:** clicking any past session opens its transcript in the chat view;
|
||||
starting a new chat and switching back and forth works without navigation.
|
||||
|
||||
---
|
||||
|
||||
## Phase 4 — Agent efficiency
|
||||
|
||||
Goal: stop re-deriving state; start each turn already oriented.
|
||||
|
||||
- Persist and replay tool evidence across turns
|
||||
([agent.go:108-127](../cmd/nomos/agent.go)): either replay `tool_use`/
|
||||
`tool_result` pairs with consistent ids, or persist a compacted per-turn
|
||||
"evidence summary" and replay that. Removes redundant re-querying.
|
||||
- Inject a compact fleet snapshot (counts by health, open signals, stale set)
|
||||
into the system prompt ([agent.go:81](../cmd/nomos/agent.go)) so the agent
|
||||
starts oriented instead of spending iterations on discovery.
|
||||
- Add prompt caching for the system prompt + tool schema (rebuilt every call
|
||||
today); revisit the default model
|
||||
([agent.go:34](../cmd/nomos/agent.go)) — evaluate a stronger default for
|
||||
fewer, better tool calls.
|
||||
- Surface per-turn iteration/token/cost in the chat UI (data already logged to
|
||||
`agent_activity`) so inefficiency is visible and measurable.
|
||||
|
||||
**Acceptance:** a 3-turn conversation about the same entity does not re-call the
|
||||
same read tools each turn; median iterations-per-answer drops.
|
||||
|
||||
---
|
||||
|
||||
## Phase 5 — Customize & inspect (CRUD)
|
||||
|
||||
Goal: manage the system from the UI, not just observe it.
|
||||
|
||||
- Entity editor (attributes, state) via existing mutation endpoints.
|
||||
- Check management from the entity detail sheet: enable/disable, edit config/
|
||||
thresholds, and **run-now** (trigger a single check pass on demand — new
|
||||
scheduler entrypoint).
|
||||
- Relationship add/remove.
|
||||
- Raw DB-row view toggle in the detail sheet for power inspection.
|
||||
|
||||
---
|
||||
|
||||
## Suggested order of work
|
||||
|
||||
1. Phase 1 backend (staleness sweep + `/entities` health/freshness) →
|
||||
Phase 1 frontend (table freshness + liveness header).
|
||||
2. Phase 2 DetailSheet + entity monitoring card.
|
||||
3. Phase 3 sessions fix (small; can slot in earlier if desired).
|
||||
4. Phase 4 agent efficiency.
|
||||
5. Phase 5 CRUD.
|
||||
|
||||
Phases 1–3 are the ones that most directly turn "the system feels dead and I
|
||||
don't trust it" into "it's alive and I can see and act on it."
|
||||
254
plans/done/2026-07-08-oikos-gaps-and-improvements.md
Normal file
254
plans/done/2026-07-08-oikos-gaps-and-improvements.md
Normal file
@@ -0,0 +1,254 @@
|
||||
# 2026-07-08 — Oikos gaps, broken things, and improvements
|
||||
|
||||
**Reviewed 2026-08-16 — Status: Completed** — many items resolved during 2026-07-11–12; remaining gaps superseded by hexagonal refactor (v0.34.0).
|
||||
|
||||
**Status:** In Progress — audited 2026-07-11, re-audited 2026-07-12 for
|
||||
drift from the `cmd/hermes`→`cmd/nomos` rename and later fixes. Done: A1
|
||||
(approval FK bug), A3 (Hermes→Nomos help text), **Section C** (toy NLU /
|
||||
silent-wrong-answer fallback — nomos now calls real `listTools()` and
|
||||
routes unmatched queries to `/chat` instead of guessing, per
|
||||
`cmd/nomos/main.go:444-465`), **D.5** (SOUL.md/actuator architecture
|
||||
mismatch — `nomos/SOUL.md:21-22,40` now accurately documents SSH via the
|
||||
`run` tool), D1 (`upsert_knowledge`), D4-partial (general `run` tool).
|
||||
Still open: A2 (notifier flooding/dedup), A4 (`resolveHost` dead code), A5
|
||||
(`queryRows` stringly-typed columns), A6 (stale `get_state_snapshot`
|
||||
description), B1-B5 (enrollment auth, fake Infisical creds, `/query`
|
||||
mesh-only auth unenforced, insecure host key checking, optional
|
||||
`caller_pubkey`), D2/D3 (no `get_approval_status`/`list_pending_approvals`/
|
||||
signal ack-resolve-mute tools), E-partial (Caddyfile placeholders still
|
||||
present; tool count now 33, documented in AGENTS.md as of 2026-07-12).
|
||||
2026-07-12 re-audit also refreshed every `cmd/hermes`→`cmd/nomos` and
|
||||
`internal/mcp/server.go` line-number citation below (the file grew from 28
|
||||
to 33 registered tools since 2026-07-11) — content/status of each finding
|
||||
unchanged, only citations moved.
|
||||
|
||||
## Goal
|
||||
|
||||
Full-project review of Oikos from two vantage points — a user interacting
|
||||
through Hermes, and an agent working through the MCP tool surface — with every
|
||||
finding verified against source (file:line), plus a prioritized fix order.
|
||||
This plan is the map; each numbered fix is small enough to land independently.
|
||||
|
||||
---
|
||||
|
||||
## A. Confirmed bugs (verified in source)
|
||||
|
||||
### A1. Approvals are never created — FK violation, errors swallowed (CRITICAL)
|
||||
|
||||
`createApproval` at `internal/mcp/server.go:970` inserts a fresh
|
||||
`uuid.NewV7()` as `approvals.entity_id`, but
|
||||
`migrations/003_operations.up.sql:41` declares
|
||||
`entity_id UUID PRIMARY KEY REFERENCES entities(id)`. The INSERT always
|
||||
violates the FK, and both `pool.Exec` errors are discarded.
|
||||
|
||||
Net effect: `request_execution` for `systemctl enable/disable` or
|
||||
`apt_upgrade` marks the execution `pending_approval`
|
||||
(`internal/mcp/server.go:311`, `:366`) and tells the agent it's queued, but no
|
||||
approval row exists → the notifier never sends a Matrix alert → the execution
|
||||
is orphaned forever. From the Hermes user's perspective, config mutations
|
||||
silently dead-end while appearing accepted.
|
||||
|
||||
Contrast: `request_execution` (server.go:286) correctly creates a companion
|
||||
`entities` row for the execution first — approvals just never got the same
|
||||
treatment.
|
||||
|
||||
**Fix:**
|
||||
- Preferred: migrate `approvals` to its own `id UUID PRIMARY KEY` (not FK'd to
|
||||
`entities`), keeping `subject_entity_id` as the entity link. Update notifier
|
||||
and `DecideApproval` queries accordingly.
|
||||
- Alternative (no migration): create a companion `entities` row like
|
||||
executions do.
|
||||
- Either way: check and log every `Exec` error in `createApproval`, and verify
|
||||
the `UPDATE executions SET approval_id = $2 WHERE entity_id = $1` column
|
||||
semantics (`executions` is also keyed by `entity_id`).
|
||||
|
||||
### A2. Matrix message-flooding vectors (`internal/notifier/notifier.go`)
|
||||
|
||||
- Initial alert is guarded by `alert_sent_at` (notifier.go:103), but the guard
|
||||
is written *after* the Matrix send (notifier.go:111). If the UPDATE fails
|
||||
after a successful send, the ticker re-sends every cycle.
|
||||
- No dedup of approvals by `(subject_entity_id, action, payload)`. Once A1 is
|
||||
fixed, every retried `request_execution` mints a new approval → one Matrix
|
||||
message each.
|
||||
- `pollReactions` (notifier.go:118) issues one Matrix relations GET per
|
||||
pending approval per 30s poll, uncapped — ignored approvals accumulate for
|
||||
their 1h lifetime and multiply API calls. It also re-dispatches decisions
|
||||
for approvals stuck `pending` (no "already acted" guard if the
|
||||
DecideApproval call errors without flipping status).
|
||||
|
||||
**Fix:** mark-then-send (or transactional outbox) for `alert_sent_at`; upsert/
|
||||
dedup open approvals on `(subject_entity_id, action, payload)`; cap + backoff
|
||||
on reaction polling; guard against re-dispatching a decision already in
|
||||
flight.
|
||||
|
||||
### A3. Hermes "help" is broken + dead code
|
||||
|
||||
`cmd/hermes/main.go:165` handles "what can you do"/"help" by calling
|
||||
`client.callTool("tools/list", nil)` — a `tools/call` for a tool literally
|
||||
named `tools/list`, which doesn't exist. The correct `listTools()` helper
|
||||
(main.go:318) is dead code, never called. **Fix:** wire `listTools()` in.
|
||||
|
||||
### A4. `resolveHost` never returns a per-entity SSH user
|
||||
|
||||
`internal/mcp/server.go:1222` (was :943 — line moved) — the named return
|
||||
`sshUser` is always `""`; the per-entity user branch is dead and everything
|
||||
relies on `sshExec`'s global default fallback. **Fix:** read the SSH user
|
||||
from entity attributes or delete the dead return to make the behavior
|
||||
honest.
|
||||
|
||||
### A5. `queryRows` stringifies every column
|
||||
|
||||
`internal/mcp/server.go:1090` (was :861 — line moved) renders all values via
|
||||
`fmt.Sprintf("%v", ...)`, so numbers, bools, timestamps, and JSON all reach
|
||||
agents as strings. **Fix:** type-preserving serialization (pass through
|
||||
pgx-native values into `json.Marshal`) — improves every read tool at once.
|
||||
|
||||
### A6. `get_state_snapshot` description is stale
|
||||
|
||||
`internal/mcp/server.go:863` (was :689 — line moved) still advertises "disk,
|
||||
drift count" — columns removed in commit 3ea43ad. **Fix:** update the
|
||||
description.
|
||||
|
||||
---
|
||||
|
||||
## B. Security gaps
|
||||
|
||||
### B1. Enrollment is unauthenticated, with a false comment
|
||||
|
||||
`internal/httpapi/server.go:111` (was :97) says "unauthenticated (IP-gated
|
||||
in handler)" but `EnrollClient` (`internal/httpapi/impl.go:1166`, was
|
||||
:1099) performs no IP check at all — the only gate is the target entity
|
||||
being in state `planned`/`provisioning`. Caddy's `@enroll` matcher bypasses
|
||||
Authentik. Anyone reaching `oikos.hubris.network` who knows (or guesses) a
|
||||
planned slug receives that node's **age private key** in the HTTP response
|
||||
body. Still open — line numbers only, substance unchanged.
|
||||
|
||||
**Fix:** enforce a real gate (mesh-CIDR check, one-time enrollment token
|
||||
minted when the entity is created, or both), and stop returning the age
|
||||
private key in the response — have the client fetch it from the secret store.
|
||||
|
||||
### B2. Fake Infisical credentials returned to enrollees
|
||||
|
||||
`internal/httpapi/impl.go:1260-1261` (was :1191-1192) returns
|
||||
`"inf_client_"+uuid` / `"inf_secret_"+uuid` — random strings wired to
|
||||
nothing. Enrolled clients hold credentials that authenticate against
|
||||
nothing. Still open — line numbers only, substance unchanged.
|
||||
**Fix:** implement `CreateMachineIdentity` in `internal/secrets/infisical.go`,
|
||||
or return no credentials and document the manual step.
|
||||
|
||||
### B3. Nomos's `/query` has no auth
|
||||
|
||||
`nomos/config.yaml:9` (was `hermes/config.yaml:9`) sets `mesh_only: true`
|
||||
but `cmd/nomos/main.go` (was `cmd/hermes/main.go`) never reads or enforces
|
||||
it — it serves any caller on :8092, who can invoke `request_execution`.
|
||||
Still open, now also tracked as C1 in
|
||||
[2026-07-11-nomos-agent-code-review.md](2026-07-11-nomos-agent-code-review.md),
|
||||
deferred by the operator. **Fix:** enforce mesh-CIDR (or bearer token) in
|
||||
the handler; fail closed.
|
||||
|
||||
### B4. SSH host keys not verified
|
||||
|
||||
`ssh.InsecureIgnoreHostKey()` at `internal/mcp/server.go:1155` (was :920).
|
||||
Still open — line number only, substance unchanged.
|
||||
**Fix:** known_hosts pinning (keys are already inventory-managed per node).
|
||||
|
||||
### B5. `list_my_secrets` enumerates all node pubkeys
|
||||
|
||||
Without `caller_pubkey`, `internal/mcp/server.go:879-883` (was :709-720)
|
||||
returns every entity that has an `age_pubkey`; nothing ties the caller to
|
||||
what it may list. Still open — line numbers only, substance unchanged.
|
||||
**Fix:** require `caller_pubkey` and scope results to the caller's
|
||||
entitlements.
|
||||
|
||||
---
|
||||
|
||||
## C. User perspective (interacting via Hermes) — RESOLVED
|
||||
|
||||
**Resolved as of the Hermes→Nomos rewrite (verified 2026-07-12).** This
|
||||
entire section described `cmd/hermes`, which no longer exists — Hermes was
|
||||
renamed and rebuilt as `cmd/nomos`, a real LLM-backed agent loop, which is
|
||||
exactly the recommendation below. `cmd/nomos/main.go:444-465` now calls the
|
||||
real `listTools()` for "help"/"what can you do", and routes unmatched
|
||||
queries to "natural language queries belong to `/chat`..." instead of
|
||||
silently falling back to `get_health_summary`. Kept below for history —
|
||||
original text unchanged.
|
||||
|
||||
- `routeQuery` NLU is hardcoded `strings.Contains`; `extractEntity`
|
||||
(`cmd/hermes/main.go:173`) recognizes only 5 services (`authentik, caddy,
|
||||
vaultwarden, gitea, immich`) plus `mac-mini`/`hubris`. Any other entity →
|
||||
"no entity found", and *any* unmatched query silently falls back to
|
||||
`get_health_summary` — wrong answers that look like answers.
|
||||
- No conversation/session context; no follow-up capability.
|
||||
- Config mutations appear accepted but silently dead-end (A1).
|
||||
|
||||
**Recommendation:** either make Hermes a real LLM-backed agent loop (Claude
|
||||
API driving the 28 MCP tools) or explicitly scope it as a structured-tool
|
||||
gateway: remove the toy NLU, make the fallback say "I don't understand this
|
||||
query; here are the tools" (via the fixed `listTools()`), and document that
|
||||
natural language belongs to the calling agent, not the gateway.
|
||||
|
||||
---
|
||||
|
||||
## D. Agent perspective (MCP tooling gaps)
|
||||
|
||||
28 tools are registered in `internal/mcp/server.go` (README says 15, AGENTS.md
|
||||
says 21 — both stale). Missing capabilities:
|
||||
|
||||
1. **No knowledge write.** AGENTS.md tells agents to register knowledge via
|
||||
`POST /api/v1/knowledge/{slug}`, but there is no MCP tool — MCP-only agents
|
||||
cannot write back what they learn. Add `upsert_knowledge`.
|
||||
2. **No entity/signal mutation.** Create/patch entity, state transitions, and
|
||||
signal ack/resolve/mute all exist in REST (`internal/httpapi/impl.go`,
|
||||
`phase3.go`) but not in MCP. Add at least signal ack/resolve/mute and a
|
||||
policy-gated entity attribute patch.
|
||||
3. **No approval visibility.** After `request_execution` returns
|
||||
`pending_approval`, an agent has no way to check or reference the approval.
|
||||
Add `get_approval_status` / `list_pending_approvals`.
|
||||
4. Execution actions limited to `restart | systemctl | pct_exec |
|
||||
apt_upgrade` — no deploy/rollback/config-edit path. Partially
|
||||
superseded: the general `run` MCP tool (D4-partial, done) covers
|
||||
arbitrary commands now; `request_execution`'s fixed enum is still there
|
||||
for the specific actions it names (see
|
||||
[2026-07-10-general-gated-execution.md](2026-07-10-general-gated-execution.md)).
|
||||
5. **RESOLVED (verified 2026-07-12).** Architecture/doc mismatch:
|
||||
`hermes/SOUL.md` claimed "no SSH access; all mutations flow through the
|
||||
actuator", but the MCP server ran `restart`/`pct_exec` synchronously over
|
||||
SSH from inside the api process. `nomos/SOUL.md:21-22,40` now accurately
|
||||
documents SSH access via the policy-gated `run` tool — matches the
|
||||
architecture the general-gated-execution plan built. No longer a
|
||||
mismatch.
|
||||
|
||||
---
|
||||
|
||||
## E. Doc drift / housekeeping
|
||||
|
||||
- **RESOLVED (verified 2026-07-12):** Tool counts. README 15 / AGENTS.md 21
|
||||
/ actual 28 was already stale by 2026-07-11 (registered tools grew to
|
||||
33) — AGENTS.md now documents all 33 with the full catalog (2026-07-12).
|
||||
- **Still open:** `compose/caddy/Caddyfile.oikos` retains literal
|
||||
`<mac-mini-mesh-ip>` placeholders (this repo's copy is a reference only —
|
||||
see [2026-07-12-wails-desktop-app.md](2026-07-12-wails-desktop-app.md)'s
|
||||
"Plan review" — the real config lives in `dtoro/caddy-conf`).
|
||||
- **RESOLVED:** `.agents/HERMES.md` renamed to `.agents/NOMOS.md`; the
|
||||
duplicate-line bug itself is still present at `.agents/NOMOS.md:11` —
|
||||
only the file citation was stale, the underlying nit is still open.
|
||||
- **RESOLVED (verified 2026-07-12):** `plans/index.md` drift — the broken
|
||||
link, TRMNL/Grimmory Active/Done mismatch, and missing `.hermes/plans/`
|
||||
entries described here are no longer present in the current
|
||||
`plans/index.md`; already fixed sometime after this plan was written.
|
||||
- **RESOLVED (verified 2026-07-12):** `plans/2026-07-05-oikos-prometheus-lxc.md`
|
||||
already self-corrected both the deleted-file references and the LXC 131
|
||||
collision in its own 2026-07-08 changelog — this bullet describes a
|
||||
pre-fix state.
|
||||
|
||||
---
|
||||
|
||||
## F. Prioritized fix order
|
||||
|
||||
1. **A1** approval FK + error handling — unblocks the entire
|
||||
approval → Matrix → execution path.
|
||||
2. **A2** notifier flooding guards — this branch's namesake.
|
||||
3. **B1/B2** enrollment security + **B3** Hermes auth.
|
||||
4. **D** MCP tool additions — approval status first, then knowledge write,
|
||||
then signal ops.
|
||||
5. **C** Hermes routing honesty + **A3-A6**, **B4/B5**, **E** drift cleanup.
|
||||
289
plans/done/2026-07-10-general-gated-execution.md
Normal file
289
plans/done/2026-07-10-general-gated-execution.md
Normal file
@@ -0,0 +1,289 @@
|
||||
# 2026-07-10 — General gated execution: from fixed actions to unlimited-but-gated
|
||||
|
||||
**Reviewed 2026-08-16 — Status: Completed** — `run` tool, chat-assent approval, and `request_execution` enum retirement shipped; auto-act revival and Layer 3 observability remain backlog.
|
||||
|
||||
**Status:** In Progress — re-audited 2026-07-14. Done: `ClassifyCommand` risk
|
||||
classifier, general `run` MCP tool, chat-assent approval (no button
|
||||
required), blast radius on approval cards, session digest, global activity
|
||||
feed (`Ops.svelte` "Executions" tab, risk-badged), Learning view
|
||||
(success-rate trend), **and now the `request_execution` enum retirement**
|
||||
(commit `60effcb`, 2026-07-14 — `run` is the only mutation tool; the legacy
|
||||
handler functions are kept as reference only, with a "DO NOT re-register"
|
||||
guard in `internal/mcp/server.go:360`). Still open: **revive auto-act** —
|
||||
`internal/actuator/actuator.go:~125` is still a literal
|
||||
`{"success": true, "message": "stub execution"}` stub (item 10). The
|
||||
`run`-gated path covers operator-initiated work end-to-end; auto-act is the
|
||||
observe→Act direction (signals triggering actions), still unimplemented.
|
||||
|
||||
## Goal
|
||||
|
||||
Make Nomos able to do **anything** needed to maintain the homelab — provision
|
||||
LXCs, deploy services, debug, restart, fix configs, investigate — without that
|
||||
capability being a fixed enum of hand-coded actions. The action space is
|
||||
unlimited; the *gate* on it is a risk classifier + operator approval, not a
|
||||
whitelist of tricks. Knowledge (the graph + runbooks) supplies the *how*; the
|
||||
agent's reasoning supplies the *what*; the classifier supplies the *may I*.
|
||||
|
||||
Operator directive (2026-07-10): **"The number of actions the agent should be
|
||||
able to do is unlimited. We need logic to gate destructive actions, but we
|
||||
should not limit what the agent can do."**
|
||||
|
||||
Chosen autonomy posture for v1: **approve-most (cautious)** — only genuinely
|
||||
read-only commands auto-run; anything that changes state requires operator
|
||||
approval. We can relax later once the classifier and ledger have earned trust.
|
||||
|
||||
Operator directive #2 (2026-07-10): **"I want to see the system come alive and
|
||||
learn and get better."** Observability is a first-class deliverable, not a
|
||||
side-effect. As a user I must be able to see, in real time: what is being
|
||||
executed, on what, and why; how it was classified and routed; what the outcome
|
||||
was; and — crucially — **what knowledge the session created** (new runbooks,
|
||||
patterns, resolved signals, ledger entries) so the system's growth is visible.
|
||||
|
||||
Operator directive #3 (2026-07-10): **approval is granted by chat assent, not a
|
||||
button.** When Nomos proposes a plan/action and the operator replies "go ahead"
|
||||
/ "yes" / "do it" in the chat, that assent *is* the approval. No separate
|
||||
Approve button for the normal case. (Destructive actions still require an
|
||||
explicit typed confirmation phrase — see Safety.)
|
||||
|
||||
## This is a realignment, not a new idea
|
||||
|
||||
[.agents/OIKOS.md](../.agents/OIKOS.md) already specifies this exact model:
|
||||
|
||||
> The classifier scores **risk class × blast radius × confidence** and routes:
|
||||
> auto-act / escalate / queue. The classifier can only *lower* autonomy relative
|
||||
> to policy, never raise it. When in doubt, escalate.
|
||||
> Act — execute through `homelab` commands or **runbooks** (never ad-hoc SSH).
|
||||
|
||||
So the target architecture is the *documented* architecture. The problem is the
|
||||
implementation diverged from it on the agent's action path.
|
||||
|
||||
## Gap analysis (grounded in code)
|
||||
|
||||
| Designed (OIKOS.md) | Actually implemented today |
|
||||
|---|---|
|
||||
| Classifier routes every action by risk × blast × confidence | [`internal/policy/classify.go`](../internal/policy/classify.go) `ClassifySignal()` only classifies **Signals** (the Observe pipeline), by entity+action-type. It is **not** called by the agent's mutation path. |
|
||||
| Agent acts through unlimited **runbooks** | Agent acts through `request_execution` with a **hard-coded enum**: `restart, systemctl, pct_exec, apt_upgrade, pct_create` ([`internal/mcp/server.go`](../internal/mcp/server.go)), each bespoke Go, gated by per-action `if`s, not the classifier. New capability = new Go + redeploy. |
|
||||
| Runbooks/skills are executable data | `skills` table + `.agents/skills/*` + `knowledge_entities` exist and are *readable* (`get_skills`), but **nothing executes a runbook**. The knowledge is inert w.r.t. action. |
|
||||
| Auto-act loop consumes classified signals and acts | [`internal/actuator/actuator.go:124`](../internal/actuator/actuator.go) is a literal `"stub execution"` — it marks work done without doing it. |
|
||||
| "Never ad-hoc SSH" | `pct_exec` **is** ad-hoc SSH (arbitrary shell in a container) and **auto-runs with no approval or classification**. |
|
||||
|
||||
Net: the elegant model exists as scaffolding (classifier, policy schema, risk
|
||||
classes, blast-radius graph walks, skills-as-data, ledger, and the
|
||||
approval+feedback plumbing hardened in the 2026-07-09/10 sessions), but the live
|
||||
agent→action path is a bag of tricks that bypasses all of it. Everything added
|
||||
in the recent LXC-deploy work (`pct_create` + DNS/VMID/template logic) made the
|
||||
bag *bigger* — reliable, but on the wrong axis.
|
||||
|
||||
**Bones that already exist and get reused:** `internal/policy` (classifier +
|
||||
`computeBlastRadius`/`blast_radius()` SQL), `risk_classes`/`action_risk` tables,
|
||||
`seeds/policy.yaml`, `executions`/`approvals`/`audit_log`, the MCP SSH machinery,
|
||||
and the inline approval + execution-status feedback loop (chat polls
|
||||
`GET /executions/{id}`).
|
||||
|
||||
## Target architecture — three layers
|
||||
|
||||
### Layer 0 — one general gated primitive (the foundation)
|
||||
|
||||
Collapse the fixed enum into essentially one tool:
|
||||
|
||||
```
|
||||
run(target, command, purpose, [declared_risk])
|
||||
```
|
||||
|
||||
- `target` — any host or LXC slug; resolves to SSH (host) or `pct exec` (LXC).
|
||||
- `command` — arbitrary shell.
|
||||
- `purpose` — the agent's stated intent (shown to the operator, feeds classify).
|
||||
- `declared_risk` — optional agent self-assessment.
|
||||
|
||||
Every call flows through:
|
||||
|
||||
1. **Classify** the command → `read_only | reversible_low | config_mutation |
|
||||
destructive`. Rule-based:
|
||||
- read-only **allowlist** (e.g. leading verb in `cat, ls, stat, journalctl,
|
||||
systemctl status|is-active, pct config|status, df, free, uptime, ip, ss,
|
||||
docker ps|logs, git status|log`) → `read_only`;
|
||||
- destructive **denylist** (`rm -rf`, `dd`, `mkfs`, `wipefs`, `pct destroy`,
|
||||
`qm destroy`, `shutdown`, `reboot`, `> /dev/`, `:(){ :|:& };:`, secret
|
||||
exfiltration, piping remote scripts to a root shell) → `destructive`;
|
||||
- anything writing state / installing / editing configs → `config_mutation`;
|
||||
- **default → escalate** (`config_mutation`) when unsure.
|
||||
The classifier may only make `declared_risk` **stricter**, never looser
|
||||
(mirrors "can only lower autonomy, never raise").
|
||||
2. **Route** (approve-most posture):
|
||||
- `read_only` → auto-run + ledger, no approval.
|
||||
- `reversible_low` / `config_mutation` → **operator approval via chat assent**
|
||||
(v1 gates all state changes; a later posture can auto-run `reversible_low`).
|
||||
- `destructive` → approval **+ typed confirmation phrase**.
|
||||
3. **Execute** (existing SSH/`pct exec`), **verify** (optional check command),
|
||||
**ledger** (`executions` + `audit_log`), **stream feedback to chat** (reuse
|
||||
the `GET /executions/{id}` polling + `InlineApproval` phases already built).
|
||||
|
||||
### Approval by chat assent (replaces the Approve button)
|
||||
|
||||
The operator is already authenticated in the chat session, so their words are
|
||||
the authorization — a separate button is redundant friction. Flow:
|
||||
|
||||
- Nomos proposes an action/plan; the gated `run` calls sit in `pending_approval`
|
||||
(created in the same turn, tied to that turn's `correlation_id`).
|
||||
- The operator's next message is checked for **assent** ("go ahead", "yes",
|
||||
"do it", "proceed", "ship it") scoped to *that* proposal. On assent, the
|
||||
pending approvals from that turn are granted and execute.
|
||||
- Mechanism: Nomos detects assent and calls an `approve_pending(correlation_id)`
|
||||
action; the backend flips the linked approvals → the existing
|
||||
`executeApprovedAction` path runs. The **grant is recorded with the exact
|
||||
operator message** that constituted assent (audit).
|
||||
- Guards: assent only applies to approvals from the immediately-preceding turn
|
||||
(no stale "yes" approving something old); ambiguous replies ("maybe",
|
||||
"later", a follow-up question) do **not** grant — Nomos re-confirms;
|
||||
**destructive** actions ignore loose assent and still require the typed
|
||||
confirmation phrase.
|
||||
- The inline UI still *shows* the pending action and its classification (so the
|
||||
operator sees what they're assenting to) and reflects the grant — but the
|
||||
primary path is "say yes," with the button demoted to an optional affordance.
|
||||
|
||||
Layer 0 alone delivers "the agent can attempt anything; state changes are gated."
|
||||
|
||||
### Layer 1 — runbooks as executable data (reliability without rigidity)
|
||||
|
||||
The hard-won procedures become **runbooks in the knowledge DB**, retrieved and
|
||||
executed step-by-step via Layer 0 — not frozen Go:
|
||||
|
||||
- `pct_create` + its DNS-self-heal / VMID-collision / template-resolution /
|
||||
locale logic becomes the canonical **"provision LXC" runbook** (parametric
|
||||
steps the agent fills in and runs through `run`). The reliability survives as
|
||||
documented, reusable steps rather than a compiled handler.
|
||||
- New capability = **new runbook (data)**, no redeploy.
|
||||
- Keep a *small* set of mechanical helpers where a shell step is genuinely
|
||||
fiddly (e.g. "pick a free cluster VMID"), exposed as callable sub-tools — but
|
||||
the flow is agent-driven, not enum-driven.
|
||||
|
||||
This is the crucial **both/and**: the general primitive is the unlimited escape
|
||||
hatch; curated runbooks are the reliable fast-path so the agent doesn't
|
||||
re-derive DNS/VMID/docker every time (the exact thing that failed repeatedly in
|
||||
the 2026-07-09 sessions).
|
||||
|
||||
### Layer 2 — learning closes the loop
|
||||
|
||||
Successful ad-hoc `run` sequences get promoted into runbooks/patterns (the
|
||||
`learning` engine + `skills` table already exist for this); the failure ledger
|
||||
informs retries. The system grows more capable **as data**, not as code.
|
||||
|
||||
## Layer 3 — Observability: watch the system come alive
|
||||
|
||||
The user must *see* the OODA loop working, not just trust it. Four surfaces,
|
||||
built on data the loop already produces (`executions`, `audit_log`, `signals`,
|
||||
`skills`, `knowledge_entities`) — the job is to make it visible, live, and
|
||||
legible, not to invent new telemetry.
|
||||
|
||||
**1. Live action feed (in the chat turn).** Every `run` renders a card as it
|
||||
happens: `target` · `purpose` · **risk badge** (green read-only / amber
|
||||
config / red destructive) · status (queued → running → ok/failed) · collapsible
|
||||
output. Streams in real time (SSE, extend the existing execution-status feed).
|
||||
The operator watches Nomos *work*, step by step, with the reasoning (`purpose`)
|
||||
and the classifier's verdict on every step.
|
||||
|
||||
**2. "What this session did" digest.** At the end of a task/turn, a summary
|
||||
card: N commands (X auto / Y assented / Z denied), entities changed (linked),
|
||||
signals resolved, and **knowledge created** — new/updated runbooks, patterns
|
||||
promoted, notes written — each linked to its record. This is the "what did the
|
||||
agent actually change and learn" answer in one glance.
|
||||
|
||||
**3. The learning view — "the system is getting better."** A dedicated page:
|
||||
runbooks and their **success-rate trend**, newly promoted skills, pattern
|
||||
confidence (Wilson bounds already computed by the learning engine), recent
|
||||
auto-acts that succeeded unattended, and a **capability timeline** ("2026-07-11:
|
||||
learned to deploy Compose stacks; success 4/4"). Growth made tangible.
|
||||
|
||||
**4. Global activity/ledger stream.** A live feed of every action across the
|
||||
fleet — command, target, classification, decision (auto / assented-by-whom),
|
||||
outcome — the audit log rendered as a heartbeat. Filterable by entity, risk,
|
||||
outcome.
|
||||
|
||||
These reuse existing tables; the work is API endpoints + SSE fan-out + Svelte
|
||||
views, plus writing knowledge-creation events into the ledger so the digest has
|
||||
something to show.
|
||||
|
||||
## Safety model (the whole point of the gate)
|
||||
|
||||
- **Default-escalate.** Nothing is *forbidden*; risky things need the operator's
|
||||
"yes." Unknown/unparseable risk → approval.
|
||||
- **Hard denylist** for catastrophic patterns → always typed confirmation, even
|
||||
if the agent declared them safe.
|
||||
- **Blast radius at approval time** — graph walk (`blast_radius()` exists):
|
||||
"this restarts caddy → 8 downstream services."
|
||||
- **Preview / dry-run** where the command supports it.
|
||||
- **Kill-switch** (`global.auto_act`, per-target `never_auto_act.*`) already
|
||||
exists; extend to a global "require approval for everything" flip.
|
||||
- **Full audit ledger** — every command, its classification, decision, actor,
|
||||
output. Non-negotiable.
|
||||
- **Scope guards** — resolve `target` to a real entity first; refuse commands
|
||||
against `destroyed`/unknown targets; cap output size (already done).
|
||||
|
||||
## Honest risks / tradeoffs
|
||||
|
||||
- Trades a small vetted surface (5 actions) for arbitrary root across the fleet,
|
||||
LLM-driven, gated only by classifier + approval. Classifying arbitrary shell
|
||||
perfectly is impossible; **default-escalate + hard denylist + always-on audit**
|
||||
is the mitigation, not perfect classification.
|
||||
- Approve-most means more operator clicks initially. Acceptable while trust is
|
||||
built; the posture is a config knob, not a rewrite.
|
||||
- Runbook-as-data can drift from reality like any doc; the ledger + verify step
|
||||
+ learning loop are the correction mechanism.
|
||||
|
||||
## Migration path (incremental, each step shippable)
|
||||
|
||||
1. **Command classifier** — extend `internal/policy` with
|
||||
`ClassifyCommand(cmd, declaredRisk) → riskClass` (allowlist/denylist/default-
|
||||
escalate + can-only-escalate rule). Unit-tested against a corpus of safe /
|
||||
mutating / catastrophic commands.
|
||||
2. **`run` tool** — new MCP tool routing classify → gate → execute → the
|
||||
existing feedback path. Ship alongside the current tools (no removal yet).
|
||||
3. **Live action feed (UI)** — render each `run` as a streaming card in chat:
|
||||
purpose, target, risk badge, status, output. This is the first "come alive"
|
||||
win and validates the SSE fan-out.
|
||||
4. **Chat-assent approval** — assent detection scoped to the last turn's
|
||||
`correlation_id` → `approve_pending`; grant records the operator's message;
|
||||
destructive still needs the typed phrase. Demote the Approve button.
|
||||
5. **Approval context** — surface risk class + blast radius + purpose inline so
|
||||
the operator sees what they're assenting to.
|
||||
6. **Session digest + activity stream (UI)** — "what this session did / created"
|
||||
card and the global ledger feed; write knowledge-creation events to the
|
||||
ledger so there's something to show.
|
||||
7. **Runbook execution** — a "provision LXC" runbook (ports the current
|
||||
`pct_create` logic) executed via `run`; validate parity with today's handler.
|
||||
8. **Learning view (UI)** — runbook success-rate trends, promoted skills,
|
||||
capability timeline.
|
||||
9. **Retire the enum** — convert remaining hard-coded actions to runbooks; make
|
||||
`request_execution` a thin deprecated alias or remove it.
|
||||
10. **Revive auto-act** — replace the actuator stub, reusing the *same*
|
||||
classifier for the Observe→Act direction (signals), still approve-most.
|
||||
|
||||
## Verification
|
||||
|
||||
- Classifier corpus test: read-only commands auto-pass; a set of known
|
||||
catastrophic commands always route to destructive+confirmation; ambiguous
|
||||
commands escalate. No command auto-runs that mutates state.
|
||||
- End-to-end: operator asks Nomos a novel task **not** in the old enum (e.g.
|
||||
"tail caddy's error log and restart it if it's flapping"); Nomos composes
|
||||
`run` calls; read-only steps auto-run and **stream as live cards**; the restart
|
||||
gates; the operator types "go ahead" and the restart executes (no button);
|
||||
ledger records each command + classification + the assent message.
|
||||
- Observability: the session ends with a digest listing what ran, what changed,
|
||||
and any knowledge created; the learning view shows the run's contribution.
|
||||
- Parity: "provision an LXC with a service" via the runbook path matches the
|
||||
reliability proven for the `pct_create` handler (free VMID, DNS, install,
|
||||
verify), then destroy.
|
||||
|
||||
## Open questions for the operator
|
||||
|
||||
- **Reversible-low posture:** keep gating restarts/syncs in v1 (chosen), or
|
||||
auto-run them once the classifier is trusted?
|
||||
- **Confirmation phrase:** per-action typed phrase for destructive, or a global
|
||||
one? (Assent covers non-destructive; destructive keeps the typed phrase.)
|
||||
- **Assent detection:** rule/keyword match, or let the model judge assent (with
|
||||
a re-confirm on ambiguity)? How strict — does "yeah do the restart but not the
|
||||
upgrade" partially grant?
|
||||
- **Runbook authorship:** operator-authored only, or may Nomos propose new
|
||||
runbooks (subject to approval) from successful ad-hoc sequences?
|
||||
- **Blast-radius threshold:** should a large blast radius force approval even for
|
||||
otherwise-reversible actions?
|
||||
349
plans/done/2026-07-11-nomos-agent-code-review.md
Normal file
349
plans/done/2026-07-11-nomos-agent-code-review.md
Normal file
@@ -0,0 +1,349 @@
|
||||
# 2026-07-11 — Nomos agent code review: gaps and improvement plan
|
||||
|
||||
**Reviewed 2026-08-16 — Status: Completed** — all actionable items (A1–A3, B1–B3, D1–D3, E, F1) resolved or deferred; C1 (unauthenticated nomos gateway) remains open.
|
||||
|
||||
**Status:** In Progress — 2026-07-11. Every finding except C1 (A1-A3, B1-B3,
|
||||
D1-D3, E, F1) is fixed, tested, and verified live against the running stack.
|
||||
C1 (unauthenticated nomos gateway) is explicitly deferred per operator
|
||||
instruction ("leave auth out for these round of fixes") — the one item
|
||||
keeping this out of `done/`.
|
||||
|
||||
- A1 `3919ec3`, B1+B2 `c5ffaec`, A3 `926969a`, D1-D3 `76f7630`,
|
||||
A2 `c390164`, B3 `6d4f6de`, F1 `11c18e8`.
|
||||
- New `internal/safego` package (B1) and `cmd/nomos/store_test.go` (A2, plus
|
||||
a regression test for the earlier plan-append fix) are the first automated
|
||||
tests for any of this package's core logic — closing part of finding E,
|
||||
though full coverage of agent.go/main.go remains future work.
|
||||
- C1 remains open — nomos's gateway (port 8092) still has no authentication.
|
||||
Revisit separately.
|
||||
|
||||
## Scope
|
||||
|
||||
A full read-through of `cmd/nomos/` (agent.go, store.go, main.go, continue.go,
|
||||
assent.go, tasks.go — 3,120 lines) plus targeted checks of its HTTP exposure,
|
||||
goroutine safety, and test coverage. Every finding below is grounded in a
|
||||
specific file:line or a runnable reproduction — two of the sharper ones
|
||||
(A1, A2) were empirically confirmed with throwaway test probes before being
|
||||
written up, not just read and assumed.
|
||||
|
||||
This is a review, not an implementation — findings are ranked by severity with
|
||||
a proposed fix per item; nothing here has been changed yet.
|
||||
|
||||
---
|
||||
|
||||
## A. Correctness bugs (confirmed, not theoretical)
|
||||
|
||||
### A1. Chat-assent word matching has real substring false positives
|
||||
|
||||
[assent.go:73-103](../cmd/nomos/assent.go). `isAssent`/`isTypedConfirmation`
|
||||
pad the message with spaces and word-boundary-check the **negation** list
|
||||
(`strings.Contains(m, " "+w+" ")`), but the **assent**/**confirm** checks use
|
||||
bare `strings.Contains(m, w)` — no word boundary at all. Confirmed live via a
|
||||
test probe:
|
||||
|
||||
- `isAssent("not sure, maybe yesterday's logs show something useful")` →
|
||||
**`true`** (`"yes"` matches inside `"yesterday"`; `"not"` alone isn't in
|
||||
`negationWords`, only the phrase `"not yet"` is).
|
||||
- `isTypedConfirmation("I haven't confirmed anything yet, let me think")` →
|
||||
**`true`** (`"confirm"` matches inside `"confirmed"`; `"haven't"` isn't in
|
||||
`negationWords`, which only has `"don't"`/`"do not"`, not other contracted
|
||||
negatives).
|
||||
|
||||
The second one is the serious half: `isTypedConfirmation` is the **sole gate
|
||||
for DESTRUCTIVE actions** ([agent.go:220-223](../cmd/nomos/agent.go)) — a
|
||||
message that merely *mentions* not having confirmed something yet can read as
|
||||
an explicit confirmation.
|
||||
|
||||
**Fix:** apply the same space-padded word-boundary check to the assent/confirm
|
||||
word lists that negation already uses. Expand `negationWords` to cover
|
||||
contracted negatives (`haven't`, `hasn't`, `isn't`, `wasn't`, `can't`,
|
||||
`won't`, `not` as a standalone word, not just `"not yet"`). Add both
|
||||
reproduced cases as permanent regression tests in `assent_test.go`.
|
||||
|
||||
### A2. Unbounded conversation history replay — no windowing, no token budget
|
||||
|
||||
[agent.go:185-207](../cmd/nomos/agent.go): every single turn (`chatWith`)
|
||||
calls `a.store.getMessages(ctx, sessionID)` — [store.go:218-239](../cmd/nomos/store.go),
|
||||
`SELECT ... WHERE session_id=$1 ORDER BY created_at ASC` with **no `LIMIT`,
|
||||
no windowing, no summarization** — and replays the *entire* history into the
|
||||
LLM call every time. `truncateToolResults` ([store.go:152-185](../cmd/nomos/store.go))
|
||||
caps each individual tool **result** at 4KB, but caps nothing else: not tool
|
||||
**args**, not the number of tool calls in one message, not the total message
|
||||
count, not total tokens.
|
||||
|
||||
This isn't theoretical — an earlier production audit (see
|
||||
[chat-sessions-improvements](done/2026-07-09-chat-sessions-improvements.md))
|
||||
found a single turn with **70 tool calls** and messages up to **106KB**. Every
|
||||
subsequent turn of a long-running or heavily-autonomous task (exactly what
|
||||
auto-continuation is built for) re-sends that ever-growing history in full.
|
||||
This is a real cost, latency, and eventual context-length-limit risk that
|
||||
compounds specifically for the tasks the system is designed to run longest.
|
||||
|
||||
**Fix:** at minimum, cap replayed history to the most recent N messages or a
|
||||
token budget, with older turns either dropped or collapsed into a short
|
||||
system-message summary (`finalSummary`'s existing one-shot summarization
|
||||
pattern, [agent.go:481-492](../cmd/nomos/agent.go), could be reused for this).
|
||||
Needs a decision on where the cutoff lives (see open questions).
|
||||
|
||||
### A3. A live turn's tool-call history is lost entirely if the client disconnects mid-stream
|
||||
|
||||
[main.go handleChat](../cmd/nomos/main.go): `toolCalls`/`finalText` accumulate
|
||||
only in local closure variables; `st.saveMessage(...)` runs exactly **once**,
|
||||
after `a.chat(...)` returns, using `ctx := r.Context()` — the *same* context
|
||||
that cancels the instant the client disconnects (Stop button, tab close,
|
||||
network blip). If `a.chat` returns early because that context was cancelled,
|
||||
the final `saveMessage` call runs with an already-cancelled context and its
|
||||
error return is never checked — the whole turn's tool-call history (already
|
||||
real: executions launched, knowledge possibly written) is silently lost from
|
||||
the persisted transcript.
|
||||
|
||||
Contrast with `resumeSession`/`continueSession` ([continue.go:96-166](../cmd/nomos/continue.go)),
|
||||
which insert a placeholder row immediately and update it after every single
|
||||
tool call — exactly the incremental-persistence pattern `handleChat` lacks.
|
||||
Verified live this session: my own Stop-button test showed the turn's actual
|
||||
tool calls (6 of them) *were* visible in the UI only because the SSE stream
|
||||
had already pushed them to the browser's in-memory store before the abort —
|
||||
none of that would have survived a page reload, since nothing was persisted.
|
||||
|
||||
**Fix:** bring `handleChat` in line with `resumeSession`'s pattern — insert a
|
||||
placeholder row before the turn starts, update it after each tool call using
|
||||
a context *not* tied to the client connection for the write itself (or at
|
||||
minimum, persist with `context.Background()` in a deferred cleanup so a
|
||||
cancelled request context doesn't take the DB write down with it).
|
||||
|
||||
---
|
||||
|
||||
## B. Robustness
|
||||
|
||||
### B1. Zero panic recovery on any background goroutine
|
||||
|
||||
Every explicitly-spawned goroutine across the agent surface has no
|
||||
`recover()`:
|
||||
|
||||
```
|
||||
cmd/nomos/main.go:78 go nAgent.runContinuationWorker(ctx)
|
||||
cmd/nomos/main.go:80 go func() { ...sweep ticker... }()
|
||||
cmd/nomos/main.go:117 go func() { ...http server... }()
|
||||
cmd/nomos/main.go:347 go a.resumeSession(context.Background(), sessionID, note)
|
||||
internal/mcp/server.go:477,495 go executeApprovedViaAPI(...)
|
||||
internal/mcp/server.go:1134 go func() { ... }()
|
||||
internal/httpapi/phase3.go:119,1456
|
||||
internal/httpapi/server.go:81,533
|
||||
```
|
||||
|
||||
`grep -rn "recover()" cmd/nomos/ internal/mcp/ internal/httpapi/` returns
|
||||
nothing. Go's default behavior for a panic in *any* goroutine — not just the
|
||||
one handling an HTTP request, which the stdlib does recover — is to crash the
|
||||
**entire process**. `runContinuationWorker` and `resumeSession` in particular
|
||||
run complex, unattended agent logic (JSON unmarshaling of model output, tool
|
||||
result parsing, map/slice indexing) with no operator watching; a single edge
|
||||
case (a malformed tool result, an unexpected nil) takes down nomos for
|
||||
**every concurrently-running task**, not just the one that hit it. This is
|
||||
more consequential post-concurrency (today's work): more simultaneous
|
||||
unattended goroutines running agent code means more surface area for one bad
|
||||
input to end everyone's session.
|
||||
|
||||
**Fix:** wrap every explicitly-spawned goroutine body in a `defer func() {
|
||||
if r := recover(); r != nil { slog.Error(...) } }()`. A small helper
|
||||
(`safeGo(func())`) would make this consistent and hard to forget at new call
|
||||
sites.
|
||||
|
||||
### B2. Auto-continuation processes its batch sequentially, one full turn at a time
|
||||
|
||||
[continue.go:58-75](../cmd/nomos/continue.go): `processContinuations` fetches
|
||||
up to 5 pending items and runs `a.continueSession(ctx, p)` for each **in a
|
||||
plain `for` loop**, in the single `runContinuationWorker` goroutine. Each
|
||||
`continueSession` is a full LLM turn that can run for minutes (10-minute
|
||||
timeout, [continue.go:134](../cmd/nomos/continue.go)). If 3 different tasks'
|
||||
executions finish in the same 4-second tick, task #3's continuation waits for
|
||||
#1 and #2 to *completely finish* first — undercutting today's whole
|
||||
concurrency effort specifically on the auto-continuation path, which is the
|
||||
mechanism autonomous multi-step tasks depend on most.
|
||||
|
||||
**Fix:** spawn each pending continuation as its own goroutine (with B1's
|
||||
panic recovery), bounded by a small semaphore if unbounded parallelism here
|
||||
is a concern.
|
||||
|
||||
### B3. No terminal state for a permanently-failed auto-continuation
|
||||
|
||||
[continue.go:162-165](../cmd/nomos/continue.go): if the resumed LLM call
|
||||
errors on both the initial attempt and its one retry, the code logs an error
|
||||
and returns — the task is left in whatever status it was in (typically
|
||||
`executing`), with no outcome set and no operator-visible signal beyond an
|
||||
inert message buried in the transcript. There's no give-up-after-N-retries or
|
||||
dead-letter marking; the task just looks silently stuck.
|
||||
|
||||
**Fix:** on final failure, call the same path `complete_task` would use to set
|
||||
`outcome='failure'` with a summary explaining the resume failed, so the task
|
||||
board reflects reality instead of showing a task that looks perpetually
|
||||
"executing."
|
||||
|
||||
---
|
||||
|
||||
## C. Security
|
||||
|
||||
### C1. Nomos's own HTTP gateway has zero authentication
|
||||
|
||||
[docker-compose.yml:144](../docker-compose.yml) publishes port 8092 directly
|
||||
(`"8092:8092"`, comment: *"mesh-published"*) and
|
||||
[Caddyfile.oikos:52-54](../compose/caddy/Caddyfile.oikos) reverse-proxies to
|
||||
it — as of the client/server split
|
||||
([2026-07-12-wails-desktop-app.md](2026-07-12-wails-desktop-app.md)), only
|
||||
from `nomos.hubris.network` now, not two routes: `/agent/*` on
|
||||
`oikos.hubris.network` was repointed to go through `api`'s own authenticated
|
||||
proxy mount instead of straight to nomos:8092, but that's `combinedAuth`
|
||||
authenticating the *hop into api*, not anything nomos itself checks — this
|
||||
finding is unaffected by that change, still fully open. `grep -n
|
||||
"Authorization\|Bearer\|auth" cmd/nomos/main.go` still returns **nothing**
|
||||
for nomos's inbound routes (nomos did gain outbound auth as *part of* the
|
||||
client/server split — it now sends `Authorization: Bearer
|
||||
$OIKOS_MCP_BEARER_TOKEN` on its own calls to `api` — but that's the opposite
|
||||
direction from this finding) — `/chat`, `/sessions`, `/sessions/{id}`
|
||||
(including `DELETE`), and `/query` have no credential check of any kind.
|
||||
Anyone who can reach the LAN or mesh network can converse with Nomos
|
||||
directly: start tasks, read/delete any session, answer pending questions,
|
||||
and — via chat-assent — approve gated executions by typing "yes" or "I
|
||||
confirm" to whatever the agent proposes, with no authentication at all. This
|
||||
is the same class of gap
|
||||
[oikos-gaps-and-improvements](2026-07-08-oikos-gaps-and-improvements.md)
|
||||
flagged for the `api`/MCP surface (items B1-B5), but specifically for nomos's
|
||||
*own* port, which doesn't sit behind `combinedAuth` the way `api`'s routes do.
|
||||
|
||||
**Fix:** put nomos's gateway behind the same auth the `api` process uses
|
||||
(shared bearer token check at minimum), or stop publishing 8092 directly and
|
||||
route all traffic through the already-authenticated `api` proxy exclusively.
|
||||
|
||||
---
|
||||
|
||||
## D. Code quality
|
||||
|
||||
### D1. Dead code: `isTaskTool` is defined, never called
|
||||
|
||||
[tasks.go:139-146](../cmd/nomos/tasks.go). The actual dispatch in
|
||||
[agent.go:370](../cmd/nomos/agent.go) calls `a.handleTaskTool(...)` directly
|
||||
and checks its `handled` return value — `isTaskTool` is unused.
|
||||
**Fix:** delete it, or use it in `buildTools`/dispatch if a cheaper
|
||||
pre-check is actually wanted.
|
||||
|
||||
### D2. N+1 query in `recordTouched`
|
||||
|
||||
[store.go:720-742](../cmd/nomos/store.go): loops over every slug found in a
|
||||
tool call's args and issues a separate `SELECT id, type FROM entities WHERE
|
||||
slug = $1` per slug. Fine for the common case (1-3 slugs) but doesn't batch
|
||||
for tool calls naming many entities.
|
||||
**Fix:** one `SELECT id, slug, type FROM entities WHERE slug = ANY($1)` for
|
||||
all collected slugs, then loop over the results in memory.
|
||||
|
||||
### D3. `complete_task`'s outcome isn't validated
|
||||
|
||||
[tasks.go:248-257](../cmd/nomos/tasks.go) declares an `enum` in the tool
|
||||
schema (`success|failure|partial`) but [store.go:428-457](../cmd/nomos/store.go)
|
||||
never checks it — an out-of-enum value (a model typo, or a weaker model not
|
||||
respecting the schema) silently persists as-is; only `"failure"` is
|
||||
special-cased (else `status="done"`), so a stray value still "completes" the
|
||||
task but with a value the frontend's status/outcome rendering doesn't
|
||||
recognize.
|
||||
**Fix:** validate against the three allowed values in `handleTaskTool` before
|
||||
calling `store.completeTask`, defaulting unrecognized values to `"partial"`
|
||||
(safer than silently treating them as `"success"`).
|
||||
|
||||
---
|
||||
|
||||
## E. Test coverage
|
||||
|
||||
**Zero automated tests exist for `agent.go`, `store.go`, `main.go`, or
|
||||
`tasks.go`.** Only `assent.go`'s and `continue.go`'s pure string-parsing
|
||||
helpers have unit tests (`assent_test.go`, `continue_test.go`) — confirmed by
|
||||
`grep -l "func Test" cmd/nomos/*.go` matching only those two files. This means
|
||||
today's session added substantial new, safety-critical logic — session-scoped
|
||||
assent/destructive windows, the `mcpClientPool`'s creation-race handling and
|
||||
eviction sweep, `proposePlan`'s replace-vs-append branching — verified only by
|
||||
live manual testing (curl + browser), with **no regression protection**
|
||||
against a future change silently reintroducing the cross-task assent bleed or
|
||||
breaking the pool's session isolation.
|
||||
|
||||
**Fix (highest-value additions first):**
|
||||
1. `store_test.go`: `proposePlan`'s append-vs-replace branch (the exact bug
|
||||
fixed earlier today) — needs a real DB (integration-style, matching
|
||||
`internal/db/integration_test.go`'s pattern) or a query-mocking layer.
|
||||
2. `main_test.go`: `mcpClientPool.get()`'s concurrent-creation race path (two
|
||||
goroutines racing to create a client for the same new session id) and
|
||||
`sweep()`'s eviction logic — these are pure in-memory logic, no DB needed,
|
||||
straightforward to unit test.
|
||||
3. `assent_test.go`: the two confirmed false-positive cases from A1.
|
||||
|
||||
---
|
||||
|
||||
## F. Efficiency (minor)
|
||||
|
||||
### F1. Tool list + fleet snapshot re-fetched every single turn
|
||||
|
||||
[agent.go:174,181](../cmd/nomos/agent.go): `buildTools` (`tools/list` MCP
|
||||
round-trip) and `fleetSnapshot` (`get_health_summary` call) both run at the
|
||||
start of **every** `chatWith` call — including auto-continuation resumes,
|
||||
which can fire many times per task. The tool list changes only on an `api`
|
||||
process restart; the fleet snapshot is a live "as of now" read, which is
|
||||
arguably the point of it, but re-fetching the *tool list* every turn is
|
||||
avoidable.
|
||||
**Fix:** cache `buildTools`' result (e.g., in `mcpClientPool`, invalidated on
|
||||
a client's re-initialize) — worth doing only if profiling shows it matters;
|
||||
low priority relative to A-C.
|
||||
|
||||
---
|
||||
|
||||
## Implementation order
|
||||
|
||||
1. **A1** (assent false positives) — smallest, highest-severity-per-line-of-
|
||||
code fix; ships with regression tests same-PR.
|
||||
2. **C1** (unauthenticated gateway) — security-critical, independent of
|
||||
everything else here.
|
||||
3. **B1** (panic recovery) — cheap, broad safety net; do before B2 touches the
|
||||
continuation worker's goroutine structure anyway.
|
||||
4. **B2** (parallel auto-continuation) — natural follow-on to B1 since it's
|
||||
restructuring the same goroutine.
|
||||
5. **A3** (incremental persistence for live turns) — moderate effort, real
|
||||
user-visible correctness gain.
|
||||
6. **D1-D3** (small cleanups) — bundle together, low risk.
|
||||
7. **A2** (history windowing) — needs a design decision (see below) before
|
||||
implementation; largest single change.
|
||||
8. **B3**, **F1** — lower urgency, do opportunistically.
|
||||
9. **E** (tests) — ideally lands alongside each fix above (A1's tests with
|
||||
A1, etc.) rather than as one giant deferred test-writing pass.
|
||||
|
||||
## Verification
|
||||
|
||||
- **A1**: the two probe cases (`isAssent` on the "yesterday" message,
|
||||
`isTypedConfirmation` on the "haven't confirmed" message) become permanent
|
||||
tests in `assent_test.go`, asserting `false` post-fix.
|
||||
- **A2**: after adding windowing, replay a session with 70+ tool calls (the
|
||||
documented production case) and confirm the message payload sent to the LLM
|
||||
stays under a fixed token/byte ceiling regardless of session length.
|
||||
- **A3**: reproduce the Stop-button-mid-turn scenario, reload the page, and
|
||||
confirm the tool calls made before the abort are still present in the
|
||||
persisted transcript (currently: they vanish).
|
||||
- **B1**: inject a deliberate panic in a test build of `resumeSession` (or a
|
||||
fault-injection flag), confirm the process survives and logs the recovered
|
||||
panic instead of exiting.
|
||||
- **C1**: confirm an unauthenticated `curl` to nomos's `/chat` from off-mesh
|
||||
is rejected once auth lands (currently: succeeds).
|
||||
- **D1-D3**: `go vet`/build clean, `complete_task` with a bogus outcome value
|
||||
now rejected or defaulted rather than silently persisted.
|
||||
|
||||
## Open questions
|
||||
|
||||
- **A2's cutoff mechanism**: a fixed N-message window, a token-budget-aware
|
||||
trim, or LLM-summarization of dropped history? Summarization preserves the
|
||||
most context but costs an extra LLM call per trim; a fixed window is
|
||||
simplest but could drop something the agent still needs mid-task. Leaning
|
||||
fixed window + summarize-on-trim as a middle ground, but this needs a
|
||||
decision before implementation, not during.
|
||||
- **C1's auth mechanism**: reuse `api`'s existing static bearer token
|
||||
(simplest, matches an existing pattern) or route everything through `api`'s
|
||||
proxy and stop publishing 8092 at all (removes the surface entirely, but
|
||||
changes the deploy topology)? Leaning the latter if nothing else on the LAN
|
||||
legitimately needs to reach nomos directly — worth confirming with the
|
||||
operator before picking.
|
||||
- **B2's concurrency bound**: unbounded goroutines-per-tick vs. a small
|
||||
semaphore? Given the continuation batch is already capped at 5 per tick
|
||||
(`pendingContinuations(ctx, 5)`), unbounded is probably fine, but worth a
|
||||
sanity check against real task-completion clustering patterns.
|
||||
161
plans/done/2026-07-14-activity-gaps.md
Normal file
161
plans/done/2026-07-14-activity-gaps.md
Normal file
@@ -0,0 +1,161 @@
|
||||
# 2026-07-14 — Session review + Activity timeline refinements
|
||||
|
||||
**Reviewed 2026-08-16 — Status: Won't do** — frontend plan superseded by web client extraction to separate repo.
|
||||
|
||||
**Status:** Planned
|
||||
|
||||
## Session analysis: `722d8878` (2026-07-14T10:45)
|
||||
|
||||
"Fleet-wide audit: check all services, identify what needs updating"
|
||||
|
||||
### What happened
|
||||
- User asked for fleet audit → agent called `set_goal` (session → executing)
|
||||
- 82 tool calls in ONE turn: 58 `run`, 8 `get_entity`, 4 `get_relations`, etc.
|
||||
- **No `propose_plan`** — no plan steps, entered executing directly
|
||||
- **No `complete_task`** — agent produced a final text response but never closed the task
|
||||
- Session stuck at `executing` with a text conclusion but no terminal state
|
||||
|
||||
### Gaps found
|
||||
|
||||
| # | Issue | Root cause |
|
||||
|---|---|---|
|
||||
| 1 | "Agent is thinking" stuck at end | `liveStatus === 'executing'` is true even after the turn ends. AgentIndicator shows generic message because there are no running activity entries to describe. |
|
||||
| 2 | No plan proposed | Model skipped `propose_plan` — possibly because it found prior knowledge and skipped to execution. `setGoal` now sets `executing` directly (our fix), which makes `propose_plan` optional — but the sidebar "Plan" section shows "No plan yet" permanently. |
|
||||
| 3 | Task never completed | Model produced final text but never called `complete_task`. The idle sweep (2 min now) will nudge, then auto-close. |
|
||||
| 4 | Approvals invisible in Activity | 58 `run` calls, many requiring approval. These show in chat via InlineApproval but NOT in the Activity timeline. The operator has to switch to Ops page to track approvals. |
|
||||
| 5 | Activity is newest-first | Timeline shows newest at top. Feels unnatural for a sequential log — bottom-scrolling with newest at bottom is more intuitive for "watching" what the agent does. |
|
||||
| 6 | Knowledge recorded not shown | The agent recorded knowledge but it doesn't appear in Activity if it came from prior knowledge entries via `get_knowledge_content`. |
|
||||
|
||||
---
|
||||
|
||||
## Plan
|
||||
|
||||
### 1. Fix "Agent is thinking" stuck indicator
|
||||
|
||||
**Root cause:** AgentIndicator shows when `active={$streaming || liveStatus === 'executing'}`. After the turn completes, `liveStatus` is still `'executing'` and there are no running entries, so the label falls through to the generic `"Agent is thinking…"` fallback.
|
||||
|
||||
**Fix:** Change the active condition to only show when there's actual work:
|
||||
```ts
|
||||
active={$streaming || $activityLog.some((e) => e.status === 'running')}
|
||||
```
|
||||
This way it shows during streaming AND when there are running tools (auto-continuation), but NOT when the session is just "executing" with no active work.
|
||||
|
||||
Also: when the last assistant message has text AND no pending tool calls, auto-hide the indicator. The `liveStatus === 'executing'` check is too broad — it covers the entire session lifetime.
|
||||
|
||||
### 2. Approvals in Activity timeline
|
||||
|
||||
**What:** InlineApproval cards show in chat but not in Activity. Every `run` that queues an execution with "requires approval" should appear as an entry in the Activity timeline.
|
||||
|
||||
**How:**
|
||||
- In `activity.ts`, detect tool results containing "requires approval" + execution ID
|
||||
- Add `type: 'approval_pending'` entries with the execution ID, target, action, and status
|
||||
- Poll execution status and update the entry (pending → approved → running → completed/failed)
|
||||
- The InlineApproval component stays in chat for the Approve/Deny buttons
|
||||
- Activity shows the full lifecycle: approval requested → approved → running → done
|
||||
|
||||
### 3. Old-to-new ordering
|
||||
|
||||
**Fix:** Remove `.sort((a, b) => b.timestamp - a.timestamp)` → change to `.sort((a, b) => a.timestamp - b.timestamp)` or no sort at all (entries are already added in chronological order).
|
||||
|
||||
This means the timeline reads top-to-bottom as the session unfolds. Currently `newest at top` means the "Goal" and "Plan" entries appear at the bottom, which is confusing.
|
||||
|
||||
### 4. Knowledge detection
|
||||
|
||||
**Fix:** Extend the knowledge detection in `activity.ts` to also catch `upsert_knowledge` calls from `tool_use` events (not just `tool_result`), so the entry appears as "running" while recording and then "done" when the result comes back.
|
||||
|
||||
### 5. Auto-hide indicator when turn ends with text
|
||||
|
||||
**Fix:** Detect when the last assistant message has text content AND there are no pending tool_use entries without matching tool_result. In that case, the turn is complete — don't show the indicator.
|
||||
|
||||
---
|
||||
|
||||
## Plan-approve-once (new policy)
|
||||
|
||||
### Problem
|
||||
|
||||
Today: agent calls `propose_plan` + `run` × 10 in the same turn. Each `run`
|
||||
queues an individual approval. Operator sees 10 "requires approval" cards.
|
||||
After operator types "yes", each one is individually approved, THEN the
|
||||
assent window opens and future calls auto-run.
|
||||
|
||||
The operator shouldn't see per-action approvals when they already approved
|
||||
the plan. The plan IS the approval. Individual actions within an approved
|
||||
plan should auto-execute.
|
||||
|
||||
### Target
|
||||
|
||||
```
|
||||
User: "audit fleet"
|
||||
Agent: "Here's my plan: 1. List LXCs 2. Check apt on each 3. Report" ← proposes plan
|
||||
[Proposed plan: 3 steps] [Approve plan?]
|
||||
User: "approved"
|
||||
Agent: ◉ Listing containers… ← auto-runs
|
||||
◉ Checking apt on lxc:jellyfin… ← auto-runs
|
||||
...
|
||||
"Done. 19 LXCs have pending updates."
|
||||
```
|
||||
|
||||
One approval for the plan. All actions within it auto-execute. No per-action
|
||||
approval cards. Only re-approve when the agent calls `propose_plan` again
|
||||
(significant plan change).
|
||||
|
||||
### How (server-side)
|
||||
|
||||
The classification logic in `internal/mcp/server.go:run()` needs to know whether
|
||||
a plan-approval-assent-window is active for this session. Currently it checks
|
||||
`autonomy_settings` for the assent window key. The change: when `propose_plan`
|
||||
is called, pre-activate the window with a "plan-proposed" state. When the
|
||||
operator approves, transition to "plan-active". `run` calls within an active
|
||||
plan window auto-execute at `config_mutation` level.
|
||||
|
||||
Key change in `store.go:proposePlan()`:
|
||||
```go
|
||||
// Pre-record a plan-proposed window so that run calls know a plan is pending approval.
|
||||
// Once approved, this becomes the full assent window.
|
||||
key := planWindowKey(agentID, sessionID)
|
||||
s.pool.Exec(ctx, `INSERT INTO autonomy_settings (key, value) VALUES ($1, 'proposed')
|
||||
ON CONFLICT (key) DO UPDATE SET value = 'proposed'`, key)
|
||||
```
|
||||
|
||||
Then in the `run` handler, check for `plan-active` OR `assent-active`:
|
||||
```go
|
||||
// If a plan window is active, this run call is part of the approved plan
|
||||
// and config_mutation commands auto-execute without individual approval.
|
||||
if planWindowActive(ctx, agentID, sessionID) { ... }
|
||||
```
|
||||
|
||||
### How (frontend)
|
||||
|
||||
- Instead of 10 individual InlineApproval cards, show ONE "Approve plan?" card
|
||||
- When approved, all queued `run` calls from the plan turn auto-grant
|
||||
- Activity timeline shows plan approval as one entry: "✓ Plan approved — 12 actions"
|
||||
- Subsequent `run` calls show with an "auto (plan)" badge instead of approval cards
|
||||
|
||||
---
|
||||
|
||||
## Implementation plan (consolidated)
|
||||
|
||||
### Phase A — Quick fixes (today)
|
||||
|
||||
| # | Fix |
|
||||
|---|---|
|
||||
| A1 | AgentIndicator: only show when `$streaming || hasRunningActivity` (not on `liveStatus === 'executing'`) |
|
||||
| A2 | Activity timeline: old-to-new ordering |
|
||||
| A3 | Knowledge entries: detect `tool_use` events for running state |
|
||||
|
||||
### Phase B — Approvals in Activity
|
||||
|
||||
| # | Fix |
|
||||
|---|---|
|
||||
| B1 | `activityLog`: add `approval_pending` / `approval_granted` / `execution_running` / `execution_done` lifecycle entries |
|
||||
| B2 | Poll execution status and update Activity entries inline |
|
||||
| B3 | InlineApproval stays in chat (needs operator action), but lifecycle tracked in Activity |
|
||||
|
||||
### Phase C — Plan-approve-once (policy change)
|
||||
|
||||
| # | Fix |
|
||||
|---|---|
|
||||
| C1 | Backend: plan window in `autonomy_settings`, checked by `run` handler |
|
||||
| C2 | Frontend: single "Approve plan?" card instead of per-action cards |
|
||||
| C3 | Backend: when plan is approved, auto-grant all pending executions from the plan turn |
|
||||
| C4 | Activity: plan approval as single timeline entry with action count badge |
|
||||
135
plans/done/2026-07-14-activity-timeline.md
Normal file
135
plans/done/2026-07-14-activity-timeline.md
Normal file
@@ -0,0 +1,135 @@
|
||||
# 2026-07-14 — Unified sidebar activity timeline
|
||||
|
||||
**Reviewed 2026-08-16 — Status: Won't do** — frontend plan superseded by web client extraction to separate repo.
|
||||
|
||||
**Status:** Planned
|
||||
|
||||
## Current state (broken)
|
||||
|
||||
The sidebar has three sections that appear/disappear independently:
|
||||
|
||||
| Section | When visible | Shows |
|
||||
|---|---|---|
|
||||
| Plan (top) | When `$planSteps.length > 0` | Plan step names with progress bar |
|
||||
| Tool activity (middle) | When `toolCount > 0` | Compact tool list, grouped by turn |
|
||||
| This session (bottom) | When `digest.total_executions > 0` | Post-hoc execution count + knowledge |
|
||||
|
||||
State changes cause sections to **pop in/out** as the agent moves between
|
||||
planning → executing → done. The "0 tools · 0 running" counter flashes
|
||||
briefly then vanishes. Tool activity appears/disappears between turns.
|
||||
|
||||
## Target: single unified timeline
|
||||
|
||||
One section, always present when a session is loaded. Every agent action
|
||||
appears as an entry in reverse-chronological order (newest at top).
|
||||
|
||||
```
|
||||
┌─ Activity ───────────────────────── ─┐
|
||||
│ │
|
||||
│ ✓ Task completed: "Upgraded 4 LXCs" │ ← newest
|
||||
│ ◉ Running: apt upgrade on lxc:dns │
|
||||
│ ✓ run: apt upgrade on lxc:gitea │ ← tool completed
|
||||
│ ✓ Verified gitea: HTTP 200 │
|
||||
│ ◉ Step 3/5 — Upgrade dns │ ← plan step running
|
||||
│ ✓ Step 2/5 — Upgrade gitea │ ← plan step done
|
||||
│ ✓ run: apt upgrade on lxc:nfs-export │
|
||||
│ ◉ Step 1/5 — Upgrade nfs-export │
|
||||
│ ✓ Knowledge recorded │
|
||||
│ 📋 Plan set: 5 steps │ ← plan proposed
|
||||
│ 🎯 Goal: Upgrade 4 low-risk LXCs │ ← goal set
|
||||
│ │ ← oldest
|
||||
└───────────────────────────────────────┘
|
||||
```
|
||||
|
||||
### Entry types
|
||||
|
||||
| Type | Icon | Example description |
|
||||
|---|---|---|
|
||||
| `goal` | 🎯 | "Audit all LXCs for updates" |
|
||||
| `plan` | 📋 | "Plan set: 5 steps" |
|
||||
| `step_start` | ◉ spinner | "Step 2/5 — Upgrade gitea" |
|
||||
| `step_done` | ✓ | "Step 2/5 — Upgrade gitea" |
|
||||
| `tool_start` | ◉ spinner | "run: Upgrade nfs-export (21 pkgs)" |
|
||||
| `tool_done` | ✓ | "run: 0 upgraded, 0 newly installed" |
|
||||
| `tool_error` | ✗ | "run: SSH handshake failed" |
|
||||
| `knowledge` | ✨ | "Recorded: How to run fleet upgrades" |
|
||||
| `complete` | ✓ | "Task completed: success" |
|
||||
| `question` | ❓ | "Asked: Which host for the LXC?" |
|
||||
| `error` | ✗ | "Auto-resume failed: context deadline exceeded" |
|
||||
|
||||
### Data source
|
||||
|
||||
Entries come from all available sources, merged and deduplicated:
|
||||
1. **`toolTimeline` store** (live tool_use/tool_result pairs)
|
||||
2. **`planSteps` store** (step status transitions)
|
||||
3. **Session digest API** (knowledge created, final outcome)
|
||||
4. **`currentTask` store** (goal, status)
|
||||
|
||||
Deduplication: when a plan step links to a tool call via `execution_id`, show
|
||||
them as one entry instead of two (e.g. "Step 3: Upgrade dns ◉ running" includes
|
||||
the tool — don't show a separate "run: apt upgrade" entry).
|
||||
|
||||
### Behavior
|
||||
|
||||
- **Always visible** when `$currentSession` is set
|
||||
- **Reverse chronological** — newest entries at top, scrolls naturally
|
||||
- **Auto-expands** the entry for the currently-running tool/step
|
||||
- **Collapses** completed entries to one line (expandable)
|
||||
- **Polls** every 3s for live updates (same as current startPolling)
|
||||
- **No flashing** — entries only change status in place (tool_start → tool_done), never removed
|
||||
- **Persists** across page navigation (rehydrated from REST on load)
|
||||
- **Empty state** when no session: "Open a session to see agent activity"
|
||||
|
||||
### What gets removed from chat
|
||||
|
||||
- **ToolCallGroup** — the compact tool counter. Tools live in the timeline now.
|
||||
- **AgentIndicator at bottom** — partially. Keep it ONLY for the initial
|
||||
"thinking" state (before any tools fire). Once the first tool fires, the
|
||||
timeline is the source of truth and the chat indicator is redundant.
|
||||
Actually: remove it entirely. The timeline IS the indicator.
|
||||
|
||||
### What stays in chat
|
||||
|
||||
- **Agent text responses** — the thinking, conclusions, reports
|
||||
- **InlineApproval cards** — approvals need operator action, must be in chat
|
||||
- **Inline tool renderers** — entity cards, health summary, etc. (informational)
|
||||
- **User messages** — obviously
|
||||
|
||||
## Implementation
|
||||
|
||||
### 1. Data layer: `activityLog` derived store
|
||||
|
||||
Add to `chat.ts`:
|
||||
|
||||
```ts
|
||||
export interface ActivityEntry {
|
||||
id: string
|
||||
type: 'goal' | 'plan' | 'step_start' | 'step_done' | 'step_failed' |
|
||||
'tool_start' | 'tool_done' | 'tool_error' |
|
||||
'knowledge' | 'complete' | 'question' | 'error'
|
||||
description: string
|
||||
detail?: string // tool result text, step detail, etc.
|
||||
timestamp: number // Date.now() when created
|
||||
seq?: number // plan step seq, for ordering
|
||||
toolName?: string // for tool entries
|
||||
status: 'running' | 'done' | 'failed'
|
||||
collapsed: boolean // initial collapsed state (true for completed)
|
||||
}
|
||||
```
|
||||
|
||||
Derived reactively from `messages`, `planSteps`, `currentTask`, and session
|
||||
digest data. Uses `$derived.by()` to recompute when any source changes.
|
||||
|
||||
### 2. New component: `ActivityTimeline.svelte`
|
||||
|
||||
Replaces all three sidebar sections. Renders `activityLog` entries as a
|
||||
vertical timeline with connecting lines.
|
||||
|
||||
### 3. Remove from chat
|
||||
|
||||
- `<ToolCallGroup>` rendered in chat
|
||||
- `<AgentIndicator>` at bottom
|
||||
|
||||
### 4. Update TaskContextPanel
|
||||
|
||||
Replace PlanProgress + SessionDigest with ActivityTimeline.
|
||||
465
plans/done/2026-07-17-codebase-review-and-cleanup.md
Normal file
465
plans/done/2026-07-17-codebase-review-and-cleanup.md
Normal file
@@ -0,0 +1,465 @@
|
||||
# 2026-07-17 — Codebase review, lint audit, and documentation maintenance
|
||||
|
||||
**Reviewed 2026-08-16 — Status: Completed** — doc/tooling fixes shipped; code refactors superseded by hexagonal refactor.
|
||||
|
||||
Status: **Report delivered** — doc/tooling fixes applied in this commit; code
|
||||
refactors listed below are actionable recommendations pending approval.
|
||||
|
||||
Scope: full review of Go (`internal/`, `cmd/`), Svelte SPA (`web/`), and all
|
||||
documentation (`README`, `AGENTS.md`, `.agents/**`, `docs/**`, `plans/**`,
|
||||
`seeds/**`). Research-only review followed by targeted doc-maintainability
|
||||
fixes. No production code was refactored in this pass.
|
||||
|
||||
Method: three parallel research passes (Go, web, docs) plus `go vet`, `go
|
||||
build`, `go test -race`, and `npm run build`. `go vet` is clean; all tests
|
||||
pass; the SPA builds with Svelte 5 warnings (listed in §B.4).
|
||||
|
||||
## A. Headline findings
|
||||
|
||||
| # | Area | Finding | Severity |
|
||||
| - | ---- | ------- | -------- |
|
||||
| A1 | Go | `internal/httpapi/phase3.go` is a 2627-line god file holding 12+ unrelated resource domains, misnamed after a project phase | High |
|
||||
| A2 | Go | `internal/mcp/server.go:67` `newServer` is a 708-line function registering 33 tools inline; no registry pattern | High |
|
||||
| A3 | Go | 17 sqlc queries are defined but never called; ~50% of DB access bypasses sqlc with raw inline SQL in `httpapi/` | High |
|
||||
| A4 | Go | Test coverage violates the documented gates: `learning` (0%, gate 80%), `actuator`, `scheduler`, `domain`, `notifier`, `knowledge` all 0% | High |
|
||||
| A5 | Web | Entire tool-renderer registry is dead — 21 files (~1.5k lines): `tool-renderers.ts`, `renderers/index.ts`, 10 `.ts` + 10 `.svelte` registrars; `getToolRenderer` is never called | High |
|
||||
| A6 | Web | No `lint`/`check`/`test` scripts in `package.json`; zero test files; `any` is pervasive in the SSE/event payload plumbing | High |
|
||||
| A7 | Docs | `.agents/domains/knowledge/schema.md` and `.agents/shared/llm-wiki.md` describe the deleted Python substrate (`bin/homelab`, `oikos/cards/`, `oikos/ledger.py`, root `inventory.yaml`) — they contradict the DB-native model in AGENTS.md / ADR 0003 | High |
|
||||
| A8 | Docs | Brittle hardcoded counts in 5 docs: "33 tools", "15 tools", "36 documents", "20 migrations", "001–011" — rot on every seed regen | Medium |
|
||||
| A9 | Build | Desktop version hardcoded `0.1.0` in `cmd/desktop/main.go:39` and `Makefile:70` while repo is at `0.7.6` — breaks the auto-update comparison | Medium |
|
||||
| A10 | Docs | 4 broken markdown links + `plans/index.md` out of sync with filesystem (4 done plans not moved, 4 entries missing) | Low |
|
||||
|
||||
## B. Go codebase
|
||||
|
||||
`go vet ./...` clean. `go build` clean. `go test -race` passes for all packages
|
||||
that have tests. 387 `.go` files, ~33k LOC.
|
||||
|
||||
### B.1 Naming & conventions — mostly idiomatic
|
||||
|
||||
- All packages lowercase single words; no casing/abbreviation inconsistency.
|
||||
- `internal/httpapi/phase3.go` — **temporal naming** (named after a project
|
||||
phase, not a domain). Contents span checks, executions, approvals, patterns,
|
||||
skills, policy, metrics, trends, agent-activity, relationships, entity-types,
|
||||
autonomy, risk-classes. Should be split into ~12 resource files.
|
||||
- `internal/httpapi/stubs.go` — 5-line file, comment-only, no declarations.
|
||||
Orphan. **Delete.**
|
||||
- `cmd/desktop/main.go` uses stdlib `log` while the rest of the codebase
|
||||
standardizes on `slog` via `internal/observability/logging.go:11`.
|
||||
|
||||
### B.2 Dead code
|
||||
|
||||
No TODO/FIXME/XXX/HACK/DEPRECATED comments anywhere. No commented-out blocks.
|
||||
No panics in non-test code. No global mutable state.
|
||||
|
||||
Dead exported symbols:
|
||||
- `internal/notifier/notifier.go:286` — `VerifyApprovalToken` has **zero call
|
||||
sites**. Truly dead. **Delete.**
|
||||
- `internal/checkdefaults/defaults.go:22,60,125,133` — `ResolveHost`,
|
||||
`ForEntityType`, `ShortSlug`, `DefaultInterval` are exported but only called
|
||||
within their own package. **Unexport.**
|
||||
|
||||
Dead file:
|
||||
- `internal/httpapi/stubs.go` — comment-only orphan. **Delete.**
|
||||
|
||||
### B.3 Dead sqlc queries (17)
|
||||
|
||||
Defined in `internal/db/queries/*.sql`, generated into `internal/db/sqlcgen/`,
|
||||
never called anywhere in the codebase:
|
||||
|
||||
| Query | File:line |
|
||||
| ----- | --------- |
|
||||
| `ListEntityRelations` | `internal/db/queries/relationships.sql:1` |
|
||||
| `ListGraphEdges` | `internal/db/queries/relationships.sql:13` |
|
||||
| `UpsertCurrentRelationship` | `internal/db/queries/relationships.sql:25` |
|
||||
| `EndCurrentRelationship` | `internal/db/queries/relationships.sql:31` |
|
||||
| `GetEntityBySlug` | `internal/db/queries/entities.sql:7` |
|
||||
| `ListEntitiesCapped` | `internal/db/queries/entities.sql:30` |
|
||||
| `GetEntityStatus` | `internal/db/queries/operations.sql:268` |
|
||||
| `ListEntityStatus` | `internal/db/queries/operations.sql:17` |
|
||||
| `UpdateSignalState` | `internal/db/queries/operations.sql:92` |
|
||||
| `InsertApproval` | `internal/db/queries/operations.sql:212` |
|
||||
| `InsertClassification` | `internal/db/queries/operations.sql:109` |
|
||||
| `InsertFeedback` | `internal/db/queries/operations.sql:156` |
|
||||
| `InsertSkill` | `internal/db/queries/operations.sql:204` |
|
||||
| `ListEntityTypes` | `internal/db/queries/ontology.sql:1` |
|
||||
| `ListRelationshipTypes` | `internal/db/queries/ontology.sql:4` |
|
||||
| `ListLifecycleDefs` | `internal/db/queries/ontology.sql:7` |
|
||||
| `WithTx` | `internal/db/sqlcgen/db.go` |
|
||||
|
||||
**Whole `relationships.sql` file is dead** — graph/relationship access is done
|
||||
via raw inline SQL in `phase3.go` and `impl.go`. Either delete the queries or
|
||||
migrate the inline SQL to use them.
|
||||
|
||||
### B.4 Pattern divergence — raw inline SQL vs sqlc
|
||||
|
||||
CONTRIBUTING §SQL says sqlc is the convention. ~50% of DB access bypasses it:
|
||||
|
||||
- `internal/httpapi/phase3.go:164,212,260,276,338,346,386,394,418,486,546,554,555,563,583` — raw `pool.Query/Exec` with inline SQL strings.
|
||||
- `internal/httpapi/dashboard.go:21,39,59,96,117,123,144` — all raw inline SQL.
|
||||
- `internal/httpapi/activity.go:81,148,185`, `learning_view.go:31,103` — raw inline SQL.
|
||||
- `internal/httpapi/server.go:204`, `sse.go:112,142` — raw SQL (`LISTEN oikos_events`).
|
||||
|
||||
This is why the 17 queries above are dead — the equivalent logic is hand-written
|
||||
inline. **Pick one DB-access pattern.** Recommendation: migrate inline SQL to
|
||||
sqlc queries (deletes the dead queries' replacements and centralizes SQL).
|
||||
|
||||
### B.5 God files & functions (>800 lines / >100 lines)
|
||||
|
||||
Files (excluding generated):
|
||||
- `internal/httpapi/phase3.go` — **2627 lines** (split by resource).
|
||||
- `internal/mcp/server.go` — **1691 lines**.
|
||||
- `internal/httpapi/impl.go` — **1639 lines**.
|
||||
- `cmd/nomos/store.go` — **1472 lines**.
|
||||
- `cmd/nomos/main.go` — 914 lines.
|
||||
- `cmd/nomos/agent.go` — 861 lines.
|
||||
- `internal/httpapi/server.go` — 842 lines.
|
||||
- `cmd/desktop/main.go` — 784 lines.
|
||||
- `internal/scheduler/scheduler.go` — 761 lines.
|
||||
|
||||
Functions (>100 lines, worst):
|
||||
- `internal/mcp/server.go:67` `newServer` — **708 lines** (33 tools inline).
|
||||
- `cmd/nomos/agent.go:187` `chatWith` — **405 lines**.
|
||||
- `internal/httpapi/phase3.go:270` `executeApprovedAction` — **356 lines**, 5+
|
||||
levels of nested switch/if, 8 duplicated `UPDATE executions SET
|
||||
status=failed` error-bail blocks.
|
||||
- `cmd/nomos/main.go:168` `handleChat` — 193 lines.
|
||||
- `cmd/desktop/main.go:124` `startOIDCServer` — 182 lines.
|
||||
- `internal/httpapi/dashboard.go:13` `GetDashboardSummary` — 160 lines.
|
||||
- `internal/httpapi/impl.go:855` `CreateEntity` — 158 lines.
|
||||
- `internal/httpapi/phase3.go:1366` `DecideApproval` — 156 lines.
|
||||
- `internal/mcp/server.go:1264` `classifyAndGate` — 154 lines.
|
||||
|
||||
### B.6 `interface{}` vs `any`
|
||||
|
||||
Module is `go 1.26.3`; `any` is preferred. 409 `any` uses vs 11 `interface{}`.
|
||||
The 11 are in `internal/mcp/server.go:1123,1131,1133,1581`,
|
||||
`internal/httpapi/phase3.go:169,182`, and tests — all untyped-JSON unmarshaling.
|
||||
**Replace with `any`** for consistency.
|
||||
|
||||
### B.7 Test coverage
|
||||
|
||||
CONTRIBUTING §Testing gates: policy + learning ≥ 80%, others ≥ 60%.
|
||||
|
||||
| Package | Tests | Status |
|
||||
| ------- | ----- | ------ |
|
||||
| `internal/learning` | 0 | ❌ violates 80% gate |
|
||||
| `internal/actuator` | 0 | ❌ mutation code, untested |
|
||||
| `internal/scheduler` | 0 | ❌ 761 lines of check logic |
|
||||
| `internal/domain` | 0 | ❌ core types |
|
||||
| `internal/notifier` | 0 | ❌ Matrix approval flow |
|
||||
| `internal/knowledge` | 0 | ❌ seed ingestion |
|
||||
| `internal/observability` | 0 | ❌ |
|
||||
| `internal/checkdefaults` | 0 | ❌ |
|
||||
| `internal/policy` | 1 | ⚠️ covers `classify.go` only |
|
||||
| `internal/db`, `httpapi`, `mcp`, `secrets`, `config`, `ontology`, `safego` | ✅ | OK |
|
||||
| `cmd/nomos` | 3 | ✅ |
|
||||
|
||||
### B.8 Generated code & migrations — clean
|
||||
|
||||
- `internal/httpapi/gen/api.gen.go` and `internal/db/sqlcgen/*.go` all carry
|
||||
`DO NOT EDIT` headers. No hand-edits detected.
|
||||
- Migrations 001–020: sequential, no gaps, no down migrations, `embed.go`
|
||||
present. ✅
|
||||
|
||||
### B.9 OpenAPI vs implementation drift
|
||||
|
||||
- `api/openapi.yaml` defines 46 paths.
|
||||
- `internal/httpapi/` implements ~40 strict handlers + ~8 manually-registered
|
||||
`chi.Get` routes (`serveRecentActivity`, `serveSessionDigest`,
|
||||
`serveKnowledgeContent`, `serveRecentKnowledge`, `serveLearningTimeline`,
|
||||
`serveLearningTrend`, `serveOIDC*`, `serveSSE`) that are **not in
|
||||
`openapi.yaml`**.
|
||||
- OpenAPI is therefore not the source of truth for ~8 routes (violates
|
||||
CONTRIBUTING §OpenAPI codegen). **Add them to `openapi.yaml`** or document
|
||||
the carve-out.
|
||||
|
||||
## C. Web SPA (`web/`)
|
||||
|
||||
`npm run build` succeeds with Svelte 5 warnings. 722 KB JS bundle (222 KB
|
||||
gzip), no code splitting.
|
||||
|
||||
### C.1 Tooling gaps — fixed in this pass + R8
|
||||
|
||||
- `package.json` had only `dev`/`build`/`preview`. **Added** `check`
|
||||
(`svelte-check`), `typecheck` (`tsc --noEmit`), and `lint` scripts, plus
|
||||
`svelte-check` + `typescript` devDeps. Run `npm install` to pick them up.
|
||||
- **R8 added:** `eslint` (flat config) + `eslint-plugin-svelte` +
|
||||
`typescript-eslint` + `globals`; `prettier` + `prettier-plugin-svelte`;
|
||||
`vitest` (jsdom env) with a sample test (`src/lib/utils.test.ts`, 6 tests).
|
||||
New scripts: `lint`, `lint:fix`, `format`, `format:check`, `test`,
|
||||
`test:watch`. Vitest config wired into `vite.config.ts` via
|
||||
`/// <reference types="vitest/config" />`.
|
||||
- **CI:** new `web` job in `.gitea/workflows/ci.yml` runs `npm ci`,
|
||||
`npm run check` (advisory), `npm run lint` (advisory),
|
||||
`npm run format:check` (advisory), `npm run test` (gate),
|
||||
`npm run build` (gate). Advisory steps use `continue-on-error: true`
|
||||
until the lint/check baseline is clean — matching the existing
|
||||
`golangci-lint` advisory pattern.
|
||||
- **Known baseline:** svelte-check reports 154 pre-existing errors (133
|
||||
files, "No Svelte configuration found in vite config" cascade — not
|
||||
caused by R8); eslint reports 126 errors + 12 warnings (unused vars,
|
||||
`@html` XSS, unused CSS); prettier reports 175 unformatted files. These
|
||||
are real findings surfaced by the new toolchain — fixing them is a
|
||||
follow-up cleanup task.
|
||||
|
||||
### C.2 Dead code — the tool-renderer registry (21 files, ~1.5k lines)
|
||||
|
||||
`src/main.ts:25` lazy-imports `./lib/renderers`, which runs `renderers/index.ts`
|
||||
calling 10 `init*()` functions that each `registerToolRenderer(...)`. But
|
||||
**`getToolRenderer` is never called anywhere**. The whole subsystem is dead:
|
||||
|
||||
- `src/lib/tool-renderers.ts`
|
||||
- `src/lib/renderers/index.ts`
|
||||
- `src/lib/renderers/{blast-radius,change-log,entity-card,entity-table,execution-status,fleet-snapshot,health-summary,knowledge-results,lxc-list,metric-chart}.ts` (10)
|
||||
- `src/lib/renderers/{BlastRadius,ChangeLog,EntityCard,EntityTable,ExecutionStatus,FleetSnapshot,HealthSummary,KnowledgeResults,LXCList,MetricChart}.svelte` (10)
|
||||
|
||||
**Either wire it up or delete all 21 files.** Note: `HealthSummary.svelte:30`
|
||||
emits a `state_referenced_locally` Svelte 5 warning — dead code generating
|
||||
lint noise.
|
||||
|
||||
### C.3 Dead components, stores, deps
|
||||
|
||||
Dead Svelte components (never imported outside self/comments):
|
||||
- `src/lib/components/ToolCallGroup.svelte`
|
||||
- `src/lib/components/PlanProgress.svelte`
|
||||
- `src/lib/components/GoalHeader.svelte` (only in a comment)
|
||||
- `src/lib/components/InlineApproval.svelte` (only in a comment)
|
||||
- `src/lib/components/SessionDigest.svelte` + its API fn `fetchSessionDigest`
|
||||
(`src/lib/api.ts:354,363`) — dead chain.
|
||||
|
||||
Dead store exports (written, never read):
|
||||
- `src/lib/stores/context.ts:10` `pendingApprovals`
|
||||
- `src/lib/stores/events.ts:18` `connectionState`
|
||||
|
||||
Dead npm deps:
|
||||
- `mode-watcher` (`package.json:16`) — 0 imports; superseded by
|
||||
`src/lib/stores/theme.svelte.ts`.
|
||||
- `@internationalized/date` (`package.json:12`) — 0 imports.
|
||||
|
||||
Naming collision: `src/lib/components/EntityTable.svelte` (live) vs
|
||||
`src/lib/renderers/EntityTable.svelte` (dead) — same filename, easy to grab
|
||||
the wrong one.
|
||||
|
||||
### C.4 Type safety
|
||||
|
||||
No `@ts-ignore`/`@ts-expect-error`. But `any` is pervasive in the SSE/event
|
||||
plumbing — defining an `OikosEvent` discriminated union would eliminate ~15
|
||||
`any` sites:
|
||||
|
||||
- `src/lib/api.ts:32,107` — `ChatEvent.data: any`, interface `data: any`
|
||||
- `src/lib/stores/chat.ts:58-59` — `ToolCallResult.args?: any; result?: any`
|
||||
- `src/lib/stores/activity.ts:73-74` — `(t.args as any)?.seq`
|
||||
- `src/lib/stores/workspace.ts:68,83,104,152` — `data: any`, `as any`, `s: any`
|
||||
- All 10 dead renderers use `(tool.result as any).data` + `as any[]`
|
||||
- `src/pages/Config.svelte:57,74` — `(window as any).wails`, `catch (e: any)`
|
||||
- `src/lib/utils.ts:45,47` — `T extends { child?: any }`
|
||||
- `vite.config.ts:18-24` — `proxy: any`, `proxyReq: any`
|
||||
|
||||
Missing return types on exported functions: `src/lib/utils.ts:4` (`cn`),
|
||||
`src/lib/config.ts:23,42,50`, `src/lib/tool-renderers.ts:11`,
|
||||
`src/lib/stores/context.ts:18`, `src/lib/stores/events.ts:23,46`,
|
||||
`src/lib/stores/chat.ts:98,185,422,448,469`, `src/lib/oidc.ts:270`.
|
||||
|
||||
### C.5 Svelte 5 idioms — mostly clean
|
||||
|
||||
- `export let`: 0. `$:` labels: 0. `on:click`: 0. `createEventDispatcher`: 0.
|
||||
`<slot>`: 0 real usage. ✅ App is cleanly on runes.
|
||||
- Mix of `svelte/store` classic stores (`stores/{activity,chat,context,events,workspace}.ts`) and `.svelte.ts` runes modules (`theme`, `is-mobile`, sidebar context). Deliberate but could be unified.
|
||||
- `src/lib/components/ActivityTimeline.svelte:103` — `<svelte:component>` is
|
||||
**deprecated in runes mode**; components are dynamic by default. Replace with
|
||||
direct `{@const Comp = icon}{<Comp .../>}` or inline.
|
||||
- `src/lib/components/DetailSection.svelte:18` — `let open = $state(defaultOpen)`
|
||||
triggers `state_referenced_locally`; wrap in `$derived`/init via `$effect` if
|
||||
reactivity to `defaultOpen` is intended.
|
||||
|
||||
### C.6 Build/config
|
||||
|
||||
- `vite.config.ts:6-14` — reads `../VERSION` or `./VERSION`; **no fallback if
|
||||
both missing** — `readFileSync('VERSION')` throws and crashes `vite
|
||||
build`/`dev` silently. Add a fallback or a build-time check.
|
||||
- `vite.config.ts:38-50` — `server.proxy` hardcodes `localhost:8090` (API) and
|
||||
`localhost:8092` (nomos). Not env-driven.
|
||||
- `vite.config.ts:31-34` — `define: { __OIKOS_VERSION__: ... }` global is used
|
||||
in `src/lib/version.ts:1` but its declaration in `vite-env.d.ts` should be
|
||||
verified.
|
||||
- Bundle warning: single 722 KB JS chunk. Add `build.rollupOptions.output.
|
||||
manualChunks` or route-level dynamic imports.
|
||||
|
||||
### C.7 Hardcoded values
|
||||
|
||||
- `src/lib/oidc.ts:117` — `http://127.0.0.1:18901/oidc/start` (desktop OIDC
|
||||
broker port). Magic number, no constant.
|
||||
- No tokens/secrets in `src/`. Auth via `localStorage`/OIDC. ✅
|
||||
- 4 `fetch()` calls, all via `apiBase(...)`. No hardcoded hosts in fetch. ✅
|
||||
|
||||
### C.8 Accessibility
|
||||
|
||||
Generally decent (aria-label, role="button", tabindex, keyboard handlers).
|
||||
Gaps:
|
||||
- `src/pages/Chat.svelte:182` — bare `×` dismiss button missing `type="button"`.
|
||||
- `src/lib/components/{SessionGraph,EntityGraph}.svelte` — SVG `<g role="button">`
|
||||
nodes keyboard-activatable but no `aria-label` (node identity not announced).
|
||||
- `src/pages/Chat.svelte:107` — scroll container has no `role="log"`/`aria-live`
|
||||
for streamed messages.
|
||||
|
||||
## D. Documentation
|
||||
|
||||
### D.1 Stale references — fixed in this pass
|
||||
|
||||
- `AGENTS.md:115-117` — **ghost of retired `request_execution`**. Second `run`
|
||||
bullet listed the retired enum actions and contradicted the retire notice
|
||||
above it. Deleted.
|
||||
- `AGENTS.md:148` — referenced `knowledge/wiki/` (does not exist); corrected to
|
||||
`archive/knowledge/`.
|
||||
- `README.md:116` — broken link to `plans/2026-07-12-wails-desktop-app.md`
|
||||
(moved to `plans/done/`). Fixed.
|
||||
- `.agents/OIKOS.md:103` — broken link to
|
||||
`../plans/2026-07-06-consolidate-oikos-control-plane-onto-mac-mini.md`
|
||||
(in `plans/done/`). Fixed.
|
||||
- `.agents/operations/commands.md:52` — broken link with extra `/archive/`
|
||||
segment. Fixed.
|
||||
- `.agents/operations/commands.md:85` — broken link to wails plan. Fixed.
|
||||
- `.agents/shared/page-templates.md:12` — listed `HERMES.md` (renamed to
|
||||
`NOMOS.md` per ADR 0012). Fixed.
|
||||
|
||||
### D.2 Brittle counts — fixed in this pass
|
||||
|
||||
Replaced hardcoded rot-prone numbers with pointers to the source of truth:
|
||||
- `AGENTS.md:45-46` — "36 documents, 6 investigations, 12 runbooks" → pointer
|
||||
to `seeds/knowledge.yaml`.
|
||||
- `AGENTS.md:165` — "33 MCP tools" → "see §3 for the current tool list".
|
||||
- `AGENTS.md:195,203` — "as of 2026-07-12" point-in-time dates removed.
|
||||
- `.agents/OIKOS.md:108` — "migrations/ (001–011)" → "(001–020, forward-only)".
|
||||
- `.agents/OIKOS.md:111-112` — duplicate brittle counts → pointer.
|
||||
- `.agents/OIKOS.md:142` — "15 MCP tools" → pointer to AGENTS.md §3.
|
||||
- `README.md:52` — "15 tools" → pointer.
|
||||
|
||||
Remaining brittle numbers (left as-is, intrinsic to evidence trail):
|
||||
- `docs/mbse/README.md` carries many counts/dates as part of its audited
|
||||
evidence trail. Recommend adding a "Last verified: YYYY-MM-DD" header to that
|
||||
file and a scheduled re-verification (see §F).
|
||||
|
||||
### D.3 Substrate docs describing deleted Python architecture — fixed in R5
|
||||
|
||||
`.agents/domains/knowledge/schema.md` and `.agents/shared/llm-wiki.md` previously described
|
||||
`bin/homelab`, `oikos/cards/`, `oikos/ledger.py`, root `inventory.yaml`, `knowledge/sources/`,
|
||||
`get_page`/`search_docs` MCP tools — none of which exist. **Rewritten** for the DB-native model
|
||||
(ADR 0003): DB is the single source of truth for both structured data and narrative knowledge;
|
||||
`seeds/*.yaml` are the bootstrap+DR manifests; `archive/knowledge/` is the frozen legacy wiki;
|
||||
MCP `search_knowledge`/`get_entity_knowledge` replace `get_page`/`search_docs`. Substrate refs in
|
||||
`.agents/shared/{writing-style,page-templates}.md` and
|
||||
`.agents/domains/operations/schema.md` swept clean. Root `inventory.yaml` marked deprecated
|
||||
(stub points to `seeds/inventory.yaml` + DB; full on-client path reconciliation deferred to R13).
|
||||
|
||||
### D.4 ADR format
|
||||
|
||||
- 0001–0015 sequential, no gaps, indexed in `docs/adr/README.md`. ✅
|
||||
- Template drift: `0013-signal-triggers.md` uses `## Overview` (no
|
||||
Context/Decision/Consequences); `0014-entity-model.md` uses numbered
|
||||
sections, no MADR template. Status-line format differs between 0001–0010/0015
|
||||
(plain) and 0011–0014 (bold split). **Normalize** (low priority — ADRs are
|
||||
immutable history; consider a formatting pass only).
|
||||
|
||||
### D.5 Plans — fixed in this pass
|
||||
|
||||
- Moved 4 "Done" 2026-07-14 plans from `plans/` to `plans/done/`
|
||||
(session-reliability-and-ux-audit, tool-timeline-sidebar,
|
||||
unified-agent-indicator, post-fix-session-remainders).
|
||||
- Added 2 missing 2026-07-14 plans + 2 missing 2026-07-15 `done/` plans to
|
||||
`plans/index.md`.
|
||||
- Updated `plans/index.md` Done table to reflect the moves.
|
||||
|
||||
### D.6 Missing docs — fixed in this pass
|
||||
|
||||
- Created `docs/index.md` (top-level docs index, per `writing-style.md` §folder
|
||||
READMEs).
|
||||
- Created `docs/operations/README.md` (operations docs index).
|
||||
|
||||
### D.7 On-client path inconsistency
|
||||
|
||||
`AGENTS.md` uses `/opt/homelab-context/`; `CLIENTS.md` and `AGENTS.md:200`
|
||||
itself use `/opt/homelab/`. **Pick one and use consistently** (recommend
|
||||
`/opt/homelab/` per `CLIENTS.md:70-71`). Deferred — touches many lines and
|
||||
the actual deployed path needs confirming against an enrolled client.
|
||||
|
||||
### D.8 Legacy root `inventory.yaml` — deprecated in R5
|
||||
|
||||
20387-byte Python-era file superseded by `seeds/inventory.yaml` on 2026-07-07. **Replaced with a
|
||||
deprecation stub** pointing to `seeds/inventory.yaml` and the DB (ADR 0003). Kept as a stub rather
|
||||
than deleted because AGENTS.md §1/§2 still point clients at `/opt/homelab-context/inventory.yaml`
|
||||
(the on-client clone path); full path reconciliation is R13. `.agents/shared/*` and
|
||||
`.agents/domains/*` references to bare `inventory.yaml` swept to `seeds/inventory.yaml` or
|
||||
qualified as `archive/knowledge/` history.
|
||||
|
||||
## E. Build & tooling
|
||||
|
||||
### E.1 `Makefile`
|
||||
|
||||
- `make build` (`BINARY := oikos`) writes to `oikos/oikos` because `oikos/`
|
||||
exists as a directory. Functionally works (gitignored) but confusing — the
|
||||
gitignore comment says `bin/oikos`. **Recommend `BINARY := bin/oikos`** or
|
||||
rename the directory.
|
||||
- `Makefile:70` `desktop-package` hardcodes `sed 's/$$(VERSION)/0.1.0/'`. Fixed
|
||||
in this pass to read from the `VERSION` file.
|
||||
- `lint` target only runs `go vet` + optional `golangci-lint`. **Recommend
|
||||
installing golangci-lint + staticcheck + govulncheck** in CI (none are
|
||||
installed locally; CI config at `.gitea/workflows/ci.yml` should be checked).
|
||||
|
||||
### E.2 Desktop version hardcode — fixed in R6
|
||||
|
||||
`cmd/desktop/main.go:39` had `version = "0.1.0"` as a const while the repo is at `0.7.8`. Per
|
||||
`CONTRIBUTING.md:54`, the auto-update feature compares against this value — so every release tag >
|
||||
0.1.0 triggered a spurious update prompt. **Fixed:** `version` is now a `var` (default
|
||||
`"0.1.0-dev"` fallback for bare `go build`), injected from the `VERSION` file at link time via
|
||||
`make desktop` (`-ldflags "-X main.version=$(cat VERSION)"`). `CONTRIBUTING.md` updated to match.
|
||||
|
||||
### E.3 `Makefile` `BINARY` collision — fixed in R6
|
||||
|
||||
`BINARY := oikos` wrote to `./oikos`, which collided with the `oikos/` directory (Go's `-o oikos`
|
||||
into a directory named `oikos` created `oikos/oikos`). **Fixed:** `BINARY := bin/oikos` (matches
|
||||
the gitignore comment); `build` target ensures `bin/` exists; `clean` removes `bin/`. Stale
|
||||
`oikos/` cruft directory removed.
|
||||
|
||||
## F. Recommendations (actionable, ordered)
|
||||
|
||||
| ID | Action | Effort | Risk |
|
||||
| -- | ------ | ------ | ---- |
|
||||
| R1 | Delete dead Go: `notifier.VerifyApprovalToken`, `httpapi/stubs.go`; unexport 4 `checkdefaults` symbols | S | Low | ✅ done (c3973e7) |
|
||||
| R2 | Delete dead web: 21-file tool-renderer registry, 5 dead components, 2 dead store exports, 2 dead npm deps | S | Low | ✅ done (c3973e7+1) |
|
||||
| R3 | Decide sqlc vs raw SQL: delete 17 dead queries OR migrate inline SQL to use them | M | Medium | ✅ done (hybrid: 8 deleted, 9 migrated) |
|
||||
| R4 | Split `phase3.go` (2627 lines) into per-resource files; refactor `newServer` (708 lines) to a tool registry | M | Medium | ✅ done |
|
||||
| R5 | Rewrite `.agents/domains/knowledge/schema.md` + `.agents/shared/llm-wiki.md` for the DB-native model; delete/deprecate root `inventory.yaml` | M | Low | ✅ done |
|
||||
| R6 | Inject desktop `version` from `VERSION` via ldflags; fix `Makefile` `BINARY` colliding with `oikos/` dir | S | Low | ✅ done |
|
||||
| R7 | Add tests for `learning` (80% gate), `actuator`, `scheduler`, `domain`, `notifier`, `knowledge` | L | Low | ✅ partial — pure unit tests added for all 6 packages; remaining coverage needs integration tests (`make test-db`) |
|
||||
| R8 | Add `eslint`+`prettier`+`vitest` to `web/`; wire `svelte-check`+`tsc` into CI; add `web/` CI job | M | Low | ✅ done |
|
||||
| R9 | Define `OikosEvent` discriminated union; eliminate ~15 `any` sites in web | S | Low | ✅ done |
|
||||
| R10 | Replace `<svelte:component>` in `ActivityTimeline.svelte:103`; fix `state_referenced_locally` warnings | S | Low | ✅ done |
|
||||
| R11 | Add the 8 manually-registered `serve*` routes to `openapi.yaml` (or document the carve-out) | S | Low | ✅ done — documented the carve-out |
|
||||
| R12 | Add `docs/mbse/README.md` "Last verified" header + scheduled re-verification; normalize ADR 0013/0014 template | S | Low |
|
||||
| R13 | Reconcile on-client path (`/opt/homelab/` vs `/opt/homelab-context/`) across AGENTS.md + CLIENTS.md | S | Low | ✅ done |
|
||||
| R14 | Install `golangci-lint`/`staticcheck`/`govulncheck` locally + in CI | S | Low | ✅ done — `.golangci.yml` config + split Makefile targets |
|
||||
|
||||
## G. Verification
|
||||
|
||||
- `go vet ./...` — clean.
|
||||
- `go build -tags timetzdata ./cmd/oikos` — clean.
|
||||
- `go test -race -short ./...` — all tested packages pass.
|
||||
- `npm run build` — succeeds with Svelte 5 warnings (listed §C.5).
|
||||
- Doc fixes: all link targets verified to exist.
|
||||
|
||||
## H. What this commit changed
|
||||
|
||||
Applied (low-risk, reversible):
|
||||
- Created this plan.
|
||||
- Fixed 7 stale/broken doc references (AGENTS.md, README.md, OIKOS.md,
|
||||
commands.md, page-templates.md).
|
||||
- Removed 8 brittle hardcoded counts/dates; replaced with pointers to source.
|
||||
- Created `docs/index.md` and `docs/operations/README.md`.
|
||||
- Moved 4 done plans to `plans/done/`; reconciled `plans/index.md`.
|
||||
- Added `check`/`typecheck`/`lint` scripts + `svelte-check` devDep to
|
||||
`web/package.json`.
|
||||
- Fixed `Makefile:70` `desktop-package` version substitution.
|
||||
- Bumped `VERSION` 0.7.6 → 0.7.7.
|
||||
|
||||
Deferred (listed as recommendations R1–R14 above): all code deletions,
|
||||
refactors, test additions, and the substrate-doc rewrite.
|
||||
305
plans/done/2026-07-20-mascot-physics-audit.md
Normal file
305
plans/done/2026-07-20-mascot-physics-audit.md
Normal file
@@ -0,0 +1,305 @@
|
||||
# 2026-07-20 — Mascot physics/window-interaction audit + improvement plan
|
||||
|
||||
**Reviewed 2026-08-16 — Status: Completed** — P0–P2 fixes implemented and deployed; P3 ideas remain open.
|
||||
|
||||
**Status:** P0–P2 implemented. P3 partially implemented: the physics-feel
|
||||
round shipped (panic-flap cycle with speed scaling + jitter, soft
|
||||
terminal-velocity drag, one-bounce impact restitution, landing skid, wall
|
||||
ricochet, squash-and-stretch impact spring, air/drag tilt, walk bob,
|
||||
impact feather-poof particles, and a new idle-selectable `hop` behavior —
|
||||
all in `behavior.ts` + `Mascot.svelte`'s render layer, no new assets).
|
||||
The remaining P3 feature ideas (investigate badges, startle-and-flee, a
|
||||
"home" spot, round radial menu v2, distinct adult art) stay open.
|
||||
|
||||
**Verification of the fixes** (re-ran this document's own instrumented
|
||||
tests against the fixed code):
|
||||
- Case A (window closes under the mascot): position now falls smoothly
|
||||
over ~2.5–3s with visible x-drift (e.g. bottom went 84→87→104→125→...→912
|
||||
across ~3.3s), instead of jumping straight to the floor in one tick.
|
||||
- Case B (window opens over a grounded mascot with a gap beneath it): the
|
||||
mascot stayed pinned to the floor (`bottom: 950`) for 2.6s straight while
|
||||
standing under an open window whose top was far above it — no snap-up at
|
||||
all.
|
||||
- Toss momentum: a fast upward-and-sideways release made the sprite keep
|
||||
*rising* for several frames after pointerup before gravity won, then fall
|
||||
with visible deceleration bumps roughly every ~550ms (the flap cycle)
|
||||
instead of a flat monotonic increase.
|
||||
- No new console errors; `npm run build` stays clean.
|
||||
|
||||
Companion to [plans/2026-07-20-desktop-mascot.md](2026-07-20-desktop-mascot.md)
|
||||
(the original scaffolding plan, now implemented) and
|
||||
[docs/mascot/README.md](../docs/mascot/README.md) (the MBSE model). This
|
||||
document is a post-implementation review: static code read of every file
|
||||
under `web/src/lib/mascot/`, plus live testing in the browser (dragging,
|
||||
opening/closing/moving windows under the mascot, the radial menu, hatching),
|
||||
including two tests instrumented with synthetic pointer events + high-frequency
|
||||
position polling to get hard timing data rather than guessing from a laggy
|
||||
screenshot loop.
|
||||
|
||||
## Summary
|
||||
|
||||
The scaffolding (registries, FSM shape, persistence, stimulus bus) is sound
|
||||
and matches the original plan's architecture. The actual **physics is where
|
||||
it falls short of feeling alive**, for one root cause plus a few smaller
|
||||
gaps:
|
||||
|
||||
**The mascot doesn't actually fall in the cases that matter most — it
|
||||
teleports.** The only code path where a real, animated fall happens is
|
||||
"user drags it into the air and lets go." Every other ground-change case
|
||||
(a window closes or moves out from under it, a window opens or moves under
|
||||
it, it walks off a window's edge) snaps its position instantly, with zero
|
||||
animation, because of one specific piece of logic in `Mascot.svelte`. This
|
||||
was proven with instrumented timing data, not just read from the source —
|
||||
see Finding 1.
|
||||
|
||||
## How this was tested
|
||||
|
||||
- Read every file in `web/src/lib/mascot/` (behavior.ts, Mascot.svelte,
|
||||
MascotLayer.svelte, state.svelte.ts, stimuli.ts, sprites.ts, render.ts,
|
||||
actions.ts, RadialMenu.svelte, NameDialog.svelte, types.ts).
|
||||
- Ran the app (`npm run dev`), hatched a chick, and interactively tested:
|
||||
drag-and-release at various heights, opening/closing/dragging a window
|
||||
under the mascot, the right-click menu (including nested Feed), plain-click
|
||||
pet, and the hatch dialog.
|
||||
- For the two timing-sensitive claims below, screenshot-based verification
|
||||
was too slow/laggy to distinguish "instant teleport" from "fast but real
|
||||
fall" — so both were re-verified with a single `javascript_exec` call that
|
||||
dispatches synthetic `PointerEvent`s to drag the mascot precisely onto a
|
||||
window, then clicks that window's close button and polls
|
||||
`canvas.getBoundingClientRect()` every ~65ms for 2+ seconds, all inside one
|
||||
script (no inter-call latency to contaminate the result).
|
||||
- `npm run build` passes with no new warnings.
|
||||
|
||||
## Findings
|
||||
|
||||
### Finding 1 (Critical) — Ground changes teleport the mascot instead of animating a fall or rise
|
||||
|
||||
**Root cause**, `web/src/lib/mascot/Mascot.svelte` `tick()` (~lines 111-131):
|
||||
every tick, `computeGroundAt(runtime.x)` recomputes the ground line, and if
|
||||
the mascot is "grounded" (not already `falling`/`dragged`) and the ground
|
||||
changed at all, this runs unconditionally:
|
||||
|
||||
```js
|
||||
if (runtime.behavior !== 'falling' && runtime.behavior !== 'dragged' &&
|
||||
runtime.y >= prevGroundY - 1 && newGround !== prevGroundY) {
|
||||
runtime.y += newGround - prevGroundY // instant, any magnitude, either direction
|
||||
}
|
||||
```
|
||||
|
||||
This was meant to make the mascot "ride along" smoothly while a window it's
|
||||
standing on is being dragged (and it does do that correctly — verified,
|
||||
see below). But it fires for *any* ground change, not just a smooth drag,
|
||||
and it runs *before* `stepMascot()`/`behavior.ts` gets a chance to notice
|
||||
"I'm now floating" and start a real `falling` behavior — so the FSM's own
|
||||
fall-detection in the `wander`/`idle` cases
|
||||
(`if (rt.y < groundY(rt) - 1) forceBehavior(rt, 'falling')`) never actually
|
||||
fires; by the time it runs, `runtime.y` has already been silently snapped to
|
||||
match.
|
||||
|
||||
**Proven case A — window closes underneath the mascot (should fall):**
|
||||
dragged the mascot onto an open window's title bar via synthetic pointer
|
||||
events (landed cleanly: sprite bottom = 96px = window top = 96px), then
|
||||
clicked the window's close button and polled position every 65ms:
|
||||
|
||||
| t (ms) | sprite bottom (px) |
|
||||
|---|---|
|
||||
| 0 (before close) | 96 |
|
||||
| 67 | **950** (floor) |
|
||||
| 132 – 2000 | 950 (unchanged) |
|
||||
|
||||
The coded physics (gravity 1400 px/s², capped at 320 px/s) would take
|
||||
**~2.8 seconds** to fall 854px. It happened in **under 67ms** — an instant
|
||||
snap, not a fall. No `falling`/`land` animation plays.
|
||||
|
||||
**Proven case B — window opens/overlaps underneath a grounded mascot
|
||||
(should NOT rise, or should climb visibly):** with the mascot standing on
|
||||
the empty desktop floor (bottom = 950px), opened the Tasks window (which
|
||||
renders top=71px, bottom=751px at that position — its underside never
|
||||
reaches the floor, leaving a ~200px gap). Within 150ms, the mascot's sprite
|
||||
bottom was already **71px** — snapped straight up onto the new window's
|
||||
title bar, 880px in under 150ms, despite the window's bottom edge (751px)
|
||||
never actually touching the mascot's original position. `computeGroundAt()`
|
||||
has no check that the candidate window is anywhere near the mascot's
|
||||
*current* position — it just returns the topmost window overlapping the
|
||||
mascot's x column, full stop, so any window opening/moving/resizing
|
||||
anywhere in that column instantly relocates the mascot to its top edge, no
|
||||
matter the vertical distance.
|
||||
|
||||
**What does work correctly:** dragging an already-mascot-bearing window
|
||||
smoothly (title-bar drag, not open/close) — the ride-along correctly
|
||||
translates the mascot's y by the same delta as the window moves, so it
|
||||
visually "stands" on the window through the drag. Also, manual drag-and-drop
|
||||
of the mascot itself (pick it up, release above ground) *does* enter a real,
|
||||
animated `falling` → `land` → `idle` sequence, because that path is driven
|
||||
entirely by `releaseFromDrag()` from the pointer handler, which isn't
|
||||
touched by the tick-level snap.
|
||||
|
||||
**Fix direction:** `computeGroundAt` needs to return the highest surface
|
||||
*at or below* the mascot's current `y* (a downward raycast from the current
|
||||
position), not the global topmost window in the column. Separately, the
|
||||
tick-level "ride along" needs to distinguish *small, continuous* deltas
|
||||
(the window carrying the mascot while being dragged — legitimate instant
|
||||
translation) from *large or discontinuous* ones (a window appearing,
|
||||
disappearing, or the mascot walking off an edge — should hand off to
|
||||
`forceBehavior(rt, 'falling')` for a downward change, and a short new
|
||||
`rising`/hop transition for an upward one, not a silent teleport in either
|
||||
direction).
|
||||
|
||||
### Finding 2 (Critical) — the one real fall is a flat, straight drop; this is the user's specific complaint
|
||||
|
||||
Even in the one path that *does* animate (manual drag-release), the fall
|
||||
itself has no attempt at flight:
|
||||
|
||||
- `falling.enter()` in `behavior.ts` hard-sets `rt.vx = 0` — zero horizontal
|
||||
drift, ever.
|
||||
- `fall-flutter`'s animation is just `jump.png` on a loop
|
||||
(`sprites.ts`) — the *name* says flutter, the physics is a monotonic
|
||||
`vy = min(TERMINAL_VY, vy + GRAVITY*dt)` capped fall, no oscillation, no
|
||||
upward impulses.
|
||||
- There's an already-defined, already-loaded `flap` animation
|
||||
(`jump.png` again, distinct `AnimName`) that **no behavior ever
|
||||
references** — it's dead weight in the registry right now.
|
||||
|
||||
This is exactly the "not always fall directly, try to fly a little" ask.
|
||||
|
||||
### Finding 3 (Critical) — no toss/throw momentum on release
|
||||
|
||||
The original plan called for tracking recent pointer deltas during a drag
|
||||
and using them to give the release a real velocity (a toss/arc). The
|
||||
shipped `onPointerUp`/`onPointerMove` in `Mascot.svelte` track no pointer
|
||||
history at all — releasing while moving fast imparts nothing; the mascot
|
||||
just drops straight down from wherever the pointer let go, same as a slow
|
||||
release.
|
||||
|
||||
### Finding 4 (Moderate) — sprite/name/bubble clip off-screen near the top edge
|
||||
|
||||
The mascot's canvas is 20×28 logical px (extra headroom above the sprite
|
||||
for the name label and reaction bubble), positioned bottom-anchored at
|
||||
`runtime.y`. When the ground is near the top of the viewport (e.g.
|
||||
standing on a window whose title bar sits close to `y=0`, which is common
|
||||
for a freshly-opened window), the canvas — and the name label, positioned
|
||||
even further above it — render partially or fully off-screen.
|
||||
Reproduced directly: standing on a window with top=32px clipped the
|
||||
sprite from `y=-52` to `y=32`, more than half invisible above the browser
|
||||
viewport.
|
||||
|
||||
### Finding 5 (Minor) — `interruptsSleep` is defined but never read
|
||||
|
||||
`stimuli.ts`'s `ReactionDef.interruptsSleep` (set `true` only on `alarmed`)
|
||||
documents an intended rule ("sleep is broken only by reactions that opt
|
||||
in"), but nothing in `MascotLayer.svelte`'s dispatch callback or
|
||||
`behavior.ts` ever reads it — every reaction unconditionally calls
|
||||
`forceBehavior(runtime, 'react', ...)` regardless of current behavior.
|
||||
Practically, this also means a reaction can visually interrupt an active
|
||||
**drag** (the sprite briefly shows a reaction animation mid-drag, though
|
||||
position tracking is unaffected since that's driven separately by the
|
||||
pointer handler) — the documented "`dragged` always wins" rule isn't
|
||||
enforced either.
|
||||
|
||||
### Finding 6 (Cosmetic / scope gap) — radial menu isn't round
|
||||
|
||||
The implementing agent deviated from the original "round, Sims-style"
|
||||
requirement to a vertical rounded-button column (documented in the plan's
|
||||
deviation note — the polar ring layout hid labels). It works correctly,
|
||||
including nesting, but it's a direct miss against what was asked for. Worth
|
||||
a deliberate decision: keep the readable column, or revisit a true ring
|
||||
with icon-only buttons + a hover/center text readout.
|
||||
|
||||
### Finding 7 (Minor) — first-hatch naming can be dismissed with no easy way back
|
||||
|
||||
`NameDialog`'s Escape handler always calls `onCancel`, which just closes it
|
||||
— on the very first hatch prompt (no `Cancel` button is shown in `hatch`
|
||||
mode, but Escape still works via the window-level listener), a user who
|
||||
hits Escape is left with an unnamed, un-hatched egg and no obvious way to
|
||||
reopen the dialog short of reloading or finding the Debug → Force hatch
|
||||
menu action.
|
||||
|
||||
## Improvement plan
|
||||
|
||||
Ordered by priority; 1–3 directly address the user's stated complaints.
|
||||
|
||||
### P0 — Fix the ground-detection/teleport bug (Finding 1)
|
||||
|
||||
1. Change `computeGroundAt(x)` to only consider a window a ground candidate
|
||||
if its top edge is **at or below** the mascot's current `y` (plus a
|
||||
small tolerance for the "about to land on it" case) — i.e. the nearest
|
||||
surface *underneath*, not the global topmost overlapping window.
|
||||
2. Replace the unconditional `tick()`-level position snap with a threshold
|
||||
check: deltas under ~4px/tick (a window being smoothly dragged with the
|
||||
mascot riding it) still translate instantly; anything larger routes
|
||||
through `forceBehavior(rt, 'falling')` (ground dropped) or a new short
|
||||
`rising` behavior (ground rose — a quick hop/flutter-up, not a snap).
|
||||
3. This also fixes the FSM's existing (currently unreachable) `wander`/`idle`
|
||||
fall-detection — once the snap isn't preempting it, that code path
|
||||
should work as originally intended.
|
||||
|
||||
### P1 — Make falling actually look like an attempt at flight (Findings 2 & 3)
|
||||
|
||||
4. Wire the unused `flap` animation into `falling`: instead of one
|
||||
continuous `fall-flutter` loop, alternate short `flap` bursts (each
|
||||
burst applies a brief small negative `vy` impulse — a wing-beat that
|
||||
measurably slows the descent for a few frames) with `fall-flutter` glide
|
||||
segments. Net effect: still descends, but in a scalloped, fluttering
|
||||
arc rather than a flat monotonic line — reads as "trying to fly, not
|
||||
quite making it" rather than "dropped like a rock."
|
||||
5. Add a small horizontal drift during `falling` (e.g. a slow sine wobble
|
||||
or a fraction of the pre-release pointer velocity — see next point) so
|
||||
the fall isn't perfectly vertical either.
|
||||
6. Track a short rolling history of pointer positions during `dragged`
|
||||
(last ~100ms of `onPointerMove` samples is enough) and derive a release
|
||||
velocity from it in `onPointerUp`; feed that into `falling`'s initial
|
||||
`vx`/`vy` instead of hard-zeroing them, so a fast toss actually arcs.
|
||||
|
||||
### P2 — Cosmetic/correctness cleanups (Findings 4, 5, 7)
|
||||
|
||||
7. Clamp the sprite's screen-space draw position (or reserve top margin on
|
||||
the surface) so the canvas/name/bubble never render above `y=0`,
|
||||
independent of where the logical ground sits.
|
||||
8. Either wire `interruptsSleep`/a drag-guard into the reaction dispatch
|
||||
path in `MascotLayer.svelte` (skip forcing `react` while
|
||||
`runtime.behavior === 'dragged'`, and gate sleep-interruption on the
|
||||
flag as documented), or remove the field if the current
|
||||
always-interrupts behavior is actually preferred — right now it's an
|
||||
unenforced contract, which is worse than either explicit choice.
|
||||
9. On first hatch, prevent the naming dialog from being fully dismissed
|
||||
without a name (or make it trivially reopenable — e.g. clicking the
|
||||
still-unnamed egg reopens it) rather than requiring a reload/debug
|
||||
menu to recover.
|
||||
|
||||
### P3 — Ideas worth considering ("cool stuff")
|
||||
|
||||
Not committed, listed for discussion:
|
||||
|
||||
- **Investigate badges**: have the mascot occasionally walk toward a
|
||||
desktop icon that currently has an unread badge (Signals, Operations)
|
||||
and peck at it curiously — a very literal, delightful expression of
|
||||
"aware of its environment" using icon positions already in
|
||||
`stores/icons.ts`.
|
||||
- **Startle-and-flee on alarm**: instead of a static `react-alarm` frame,
|
||||
have the `alarmed` reaction actually scurry the mascot a short distance
|
||||
(reuse `wander`-style motion) before settling, more visceral than a
|
||||
still reaction sprite.
|
||||
- **A "home" spot**: remember a preferred idle location (e.g. near its
|
||||
hatch point or a favorite window) and occasionally wander back to it,
|
||||
giving its roaming a sense of place rather than pure randomness.
|
||||
- **True round radial menu v2**: revisit Finding 6 with icon-only buttons
|
||||
on an actual ring and a text label in a tooltip/center readout on
|
||||
hover/focus — closer to the original ask while keeping labels legible
|
||||
(the problem the first attempt hit).
|
||||
- **Distinct adult sprite** (already flagged as deferred polish in the
|
||||
original plan's deviation note) — currently chick and adult share art.
|
||||
|
||||
## Verification (once fixed)
|
||||
|
||||
- Re-run this document's two instrumented tests (drag-onto-window-then-close;
|
||||
open-window-over-grounded-mascot) and confirm the position samples show a
|
||||
smooth multi-frame transition instead of a single-tick jump.
|
||||
- Manually: drag the mascot up and release with a fast flick — confirm it
|
||||
arcs/drifts rather than dropping straight down, and that `flap` frames
|
||||
visibly appear during the descent.
|
||||
- Stand the mascot on a window, drag that window so its title bar approaches
|
||||
`y=0` — confirm the sprite/name/bubble stay on-screen.
|
||||
- Trigger a reaction (e.g. force an `eureka`) while mid-drag — confirm the
|
||||
sprite keeps showing the `dragged` animation, not the reaction, until
|
||||
released (if Finding 5 is fixed by enforcing the guard).
|
||||
- `npm run build` stays clean.
|
||||
649
plans/done/2026-07-21-frontend-os-apps-architecture.md
Normal file
649
plans/done/2026-07-21-frontend-os-apps-architecture.md
Normal file
@@ -0,0 +1,649 @@
|
||||
# Frontend as OS + Apps: architecture audit & refactor plan
|
||||
|
||||
**Reviewed 2026-08-16 — Status: Won't do** — frontend extracted to dtoro/oikos-web; plan should be re-evaluated there.
|
||||
|
||||
> **Status:** Planned
|
||||
> **Stakeholders:** Operator, Nomos
|
||||
> **Confidence:** Verified (direct code audit against `web/src/` as of 2026-07-21)
|
||||
|
||||
---
|
||||
|
||||
## Summary
|
||||
|
||||
The Oikos frontend is already built on an implicit OS + Apps metaphor — a
|
||||
desktop surface, floating windows, a taskbar, and a registry of
|
||||
independently-rendered apps. This plan makes that metaphor **explicit**,
|
||||
strengthens the contracts between Base OS and Apps, refactors the mascot
|
||||
into a proper App, and lays out the extensibility path for dynamic app
|
||||
installation without touching shell code.
|
||||
|
||||
The current codebase is remarkably close. The audit found one structural
|
||||
gap (mascot is hardcoded into the shell, not a registry App) and three
|
||||
contract weaknesses (positional content resolution, icon store assumes a
|
||||
static registry, no stable OS-service contract for Apps). Fixing them
|
||||
requires no architectural rewrite — the bones are correct.
|
||||
|
||||
---
|
||||
|
||||
## 1. Audit: what we have today
|
||||
|
||||
### 1.1 The implicit OS layer (exists, undocumented)
|
||||
|
||||
| Service | File | Role |
|
||||
|---------|------|------|
|
||||
| **Window Manager** | `lib/stores/windows.ts:19-31` | wmkit manager + desktop + persist. Single-instance, global. |
|
||||
| **Desktop Surface** | `components/desktop-shell/Desktop.svelte` | Full-viewport shell: background, icons, launcher, windows, mascot, taskbar. |
|
||||
| **Window Layer** | `components/desktop-shell/WindowLayer.svelte` | Content resolver: maps window ID → component. z-40. |
|
||||
| **Taskbar** | `components/desktop-shell/Taskbar.svelte` | Window buttons + tray. Renders from `wmState.order`. |
|
||||
| **Icon Grid** | `lib/stores/icons.ts` | Column/row grid, drag-to-reorder, localStorage persistence. |
|
||||
| **Task Launcher** | `components/desktop-shell/TaskLauncher.svelte` | Centered text input → new task window. |
|
||||
| **Auth Gate** | `App.svelte` | Config screen vs. Desktop. Token check, OIDC init, context/SSE subscribe. |
|
||||
| **Session Windows** | `components/SessionChatWindow.svelte` | Per-session chat window, splitpanes layout. |
|
||||
| **New Task Window** | `components/desktop-shell/NewTaskChat.svelte` | Singleton compose window. |
|
||||
| **Entity Windows** | `components/EntityDetailContent.svelte` | Entity detail (bare slug window IDs). |
|
||||
| **Legacy Hash Routes** | `App.svelte:17-39` | Backward compat for old `#/kb`, `#/entity/<slug>` bookmarks. |
|
||||
|
||||
The shell has **no hardcoded app list** — `Desktop.svelte:90` reads `APPS`
|
||||
from the registry, `WindowLayer.svelte:36-37` resolves app windows through
|
||||
`appById`, `Taskbar.svelte:32` resolves icons the same way. Adding an app
|
||||
is one entry in `apps.ts`.
|
||||
|
||||
### 1.2 The App Registry (exists, nearly complete)
|
||||
|
||||
**File:** `lib/apps.ts` (130 lines)
|
||||
**Interface:** `AppDef` — id, title, icon (Lucide Component), component
|
||||
(Svelte Component), width, height, minWidth, minHeight, optional badge
|
||||
function.
|
||||
**Window namespacing:** `app:<id>` (`apps.ts:122`) — distinct from
|
||||
`session:<id>`, `new-task`, and bare entity slugs.
|
||||
|
||||
**Current apps (7):**
|
||||
|
||||
| ID | Page Component | Badge? |
|
||||
|----|---------------|--------|
|
||||
| `tasks` | `pages/Overview.svelte` | — |
|
||||
| `kb` | `pages/KnowledgeBase.svelte` | — |
|
||||
| `ops` | `pages/Ops.svelte` | approvals_pending |
|
||||
| `signals` | `pages/Signals.svelte` | open signal count |
|
||||
| `knowledge` | `pages/Knowledge.svelte` | — |
|
||||
| `learning` | `pages/Learning.svelte` | — |
|
||||
| `settings` | `pages/Settings.svelte` | — |
|
||||
|
||||
**What works:**
|
||||
|
||||
- Data-driven. One array → three surfaces auto-render.
|
||||
- Namespaced window IDs prevent collisions with session/entity windows.
|
||||
- Single-instance enforcement (double-click focuses, never duplicates).
|
||||
- Badge system: pure function over `DashboardSummary`, consumed by icon +
|
||||
taskbar.
|
||||
- Tested (`apps.test.ts`): unique IDs, positive sizes, `appById` index,
|
||||
round-trips.
|
||||
- Orphan cleanup: `WindowLayer.svelte:25-30` closes persisted windows whose
|
||||
app was removed from the registry.
|
||||
|
||||
**What's missing from the AppDef contract:**
|
||||
|
||||
1. **No stable OS-service surface.** Apps reach into the OS by importing
|
||||
arbitrary `$lib` modules (`openEntityWindow` from `windows.ts`,
|
||||
`summary` from `context.ts`). It works because apps are compiled in, but
|
||||
there is no documented boundary between "stable OS API an App may use"
|
||||
and "shell internals that happen to be exported." Phase 3 (installed
|
||||
third-party apps) needs that boundary to exist first.
|
||||
2. **No docked/overlay app kind.** An app that renders *on* the desktop
|
||||
(above windows, no titlebar, no window at all) has no representation in
|
||||
the contract — which is exactly why the mascot is hardcoded.
|
||||
|
||||
### 1.3 The Mascot: embedded, not an app
|
||||
|
||||
**Files:** `lib/mascot/` (12 files, ~2.8k lines)
|
||||
**Integration:** `Desktop.svelte:105` — hardcoded `<MascotLayer />` at z-45,
|
||||
after WindowLayer and before Taskbar.
|
||||
|
||||
**Key facts that shape the refactor (verified):**
|
||||
|
||||
- `MascotLayer.svelte` takes **no props**. It creates the `MascotRuntime`
|
||||
per mount, seeds position from the persisted model, and attaches the
|
||||
stimulus bus itself (`MascotLayer.svelte:38-61`, comment at line 6-7).
|
||||
- The persistent model (stage, name, happiness, xp, **lastPos**) is
|
||||
module-scoped in `state.svelte.ts` and survives unmount/remount.
|
||||
- The sprite `Image` cache is module-scoped in `sprites.ts` — remounts do
|
||||
not re-fetch the 19 PNG sheets.
|
||||
- The stimulus bus subscribes to global stores (`focusedSessionId` from
|
||||
`windows.ts`, per-session factories from `chat.ts`/`workspace.ts`) — no
|
||||
dependency on how MascotLayer is mounted.
|
||||
|
||||
**Consequence:** hiding the mascot = `{#if visible}<MascotLayer />{/if}`.
|
||||
State, sprites, and position all restore naturally. No `keepAlive`
|
||||
machinery is needed.
|
||||
|
||||
### 1.4 Three contract weaknesses
|
||||
|
||||
#### Weakness 1: Positional content resolution
|
||||
|
||||
`WindowLayer.svelte:70-79` resolves content by checking ID patterns in a
|
||||
hardcoded order:
|
||||
|
||||
```svelte
|
||||
{#if id.startsWith(SESSION_PREFIX)}
|
||||
<SessionChatWindow ... />
|
||||
{:else if id === NEW_TASK_WINDOW_ID}
|
||||
<NewTaskChat />
|
||||
{:else if app}
|
||||
<app.component />
|
||||
{:else}
|
||||
<EntityDetailContent ... />
|
||||
{/if}
|
||||
```
|
||||
|
||||
A new window category must be inserted at the right position in this chain.
|
||||
Works today because prefixes are mutually exclusive by construction, but
|
||||
it's a landmine: add `'lxc:'` container consoles or `'log:'` viewers and
|
||||
you're editing shell internals.
|
||||
|
||||
#### Weakness 2: Icon store snapshots the registry at module load
|
||||
|
||||
`icons.ts:23` builds default positions from `APPS`, and `icons.ts:48`
|
||||
freezes an `appIds` set used to filter persisted positions in `load()`.
|
||||
Both evaluate **once at import time**. A late-registering app (lazy load,
|
||||
Phase 2+) would have its persisted position silently dropped by the
|
||||
`load()` filter — the merge-over-defaults logic only helps apps that were
|
||||
already in `APPS` when the module first evaluated.
|
||||
|
||||
#### Weakness 3: Window chrome is fully shell-owned, with no extension point
|
||||
|
||||
Every window gets the same titlebar (`WindowLayer.svelte:40-67`): drag
|
||||
handle, title, minimize/maximize/close. Correct default — apps should not
|
||||
draw their own chrome — but there is no sanctioned way for an app to
|
||||
contribute a titlebar affordance (e.g. Tasks might want an inline "New
|
||||
task" button). **Decision: document as a designed extension point, defer
|
||||
implementation until an app actually needs it** (see §2.5). Not a Phase 1
|
||||
deliverable.
|
||||
|
||||
---
|
||||
|
||||
## 2. The OS + Apps model
|
||||
|
||||
### 2.1 Metaphor
|
||||
|
||||
```
|
||||
┌──────────────────────────────────────────────────┐
|
||||
│ Auth Gate (App.svelte) │
|
||||
│ ┌──────────────────────────────────────────────┐│
|
||||
│ │ Desktop Surface ││
|
||||
│ │ ┌─────────────┐ ┌─────────────┐ ││
|
||||
│ │ │ App Window │ │ App Window │ z-40 ││
|
||||
│ │ │ (Tasks) │ │ (Signals) │ ││
|
||||
│ │ └─────────────┘ └─────────────┘ ││
|
||||
│ │ ┌──────────────────────┐ ││
|
||||
│ │ │ Docked Apps (Cluck) │ z-45, no chrome ││
|
||||
│ │ └──────────────────────┘ ││
|
||||
│ │ ┌──────┐ ┌──────┐ ┌──────┐ z-0 ││
|
||||
│ │ │ Icon │ │ Icon │ │ Icon │ ││
|
||||
│ │ └──────┘ └──────┘ └──────┘ ││
|
||||
│ └──────────────────────────────────────────────┘│
|
||||
│ ┌──────────────────────────────────────────────┐│
|
||||
│ │ Taskbar [Tasks] [Signals] 🎨 ⚙ v0.9 ││
|
||||
│ └──────────────────────────────────────────────┘│
|
||||
└──────────────────────────────────────────────────┘
|
||||
|
||||
Base OS = Auth Gate + Desktop Surface + Window Manager + Taskbar
|
||||
+ Icon Grid + Docked Layer + OS-service surface
|
||||
Apps = Tasks, KB, Ops, Signals, Knowledge, Learning, Settings, Cluck
|
||||
```
|
||||
|
||||
### 2.2 App kinds
|
||||
|
||||
Two kinds, distinguished by one flag:
|
||||
|
||||
| Kind | Window | Titlebar | Taskbar button | Opened by |
|
||||
|------|--------|----------|----------------|-----------|
|
||||
| **Windowed** (default) | wmkit floating window | Yes | Yes (automatic) | `openAppWindow(id)` → `wm.open()` |
|
||||
| **Docked** (`docked: true`) | None — renders on the Docked Layer | No | No | `openAppWindow(id)` → toggles visibility |
|
||||
|
||||
Docked apps are **not** wmkit citizens. They render in a dedicated layer
|
||||
above the window layer, their visibility is a persisted boolean, and
|
||||
clicking their desktop icon toggles show/hide. They never appear in the
|
||||
taskbar because they never enter `wmState.order`.
|
||||
|
||||
### 2.3 The App contract
|
||||
|
||||
```typescript
|
||||
interface AppDef {
|
||||
// Identity (required)
|
||||
id: string // unique; window IDs are "app:<id>"
|
||||
title: string // desktop icon label + window titlebar
|
||||
icon: Component // Lucide icon (desktop icon + taskbar)
|
||||
component: Component // Svelte component; receives NO props
|
||||
|
||||
// Kind
|
||||
docked?: boolean // true = Docked Layer app, no window (default false)
|
||||
|
||||
// Window geometry — required for windowed apps, forbidden for docked apps
|
||||
width?: number
|
||||
height?: number
|
||||
minWidth?: number
|
||||
minHeight?: number
|
||||
|
||||
// Behavior (all optional)
|
||||
badge?: (summary: DashboardSummary | null) => number
|
||||
noIcon?: boolean // true = registered but no desktop icon
|
||||
}
|
||||
```
|
||||
|
||||
**Validation rules** (enforced by `apps.test.ts`, not runtime checks):
|
||||
|
||||
- `id` unique, non-empty.
|
||||
- Windowed apps: `width`/`height` present and positive.
|
||||
- Docked apps: `width`/`height` absent (geometry is meaningless without a
|
||||
window).
|
||||
- Every app has an icon component (even `noIcon` apps — the taskbar and
|
||||
future surfaces need it).
|
||||
|
||||
**Design decisions, and why:**
|
||||
|
||||
- **No `keepAlive`.** Module-scoped state (mascot model, sprite cache)
|
||||
already survives unmount. If a future app needs close-to-hide semantics,
|
||||
that's a wmkit feature request, not an AppDef field.
|
||||
- **No `noTaskbar`.** Docked apps never reach the taskbar; windowed apps
|
||||
always should. A windowed app with no taskbar button is an orphan the
|
||||
operator can't find.
|
||||
- **No lifecycle hooks in the contract.** Svelte's own `onMount`/`onDestroy`
|
||||
already fire on window open/close. A shell-level `onRegister` is only
|
||||
meaningful once apps register dynamically — deferred to Phase 3, where
|
||||
it becomes the permission handshake.
|
||||
- **Apps receive no props.** The component is the app. It imports OS
|
||||
services (§2.4) directly. This keeps the shell→app edge one-way and
|
||||
trivially mockable.
|
||||
|
||||
### 2.4 The OS-service surface (AppOS)
|
||||
|
||||
The stable set of `$lib` exports an App may import. Everything else in
|
||||
`$lib` is shell-internal and may change without notice. This is a
|
||||
**documentation contract** today (apps are compiled in); it becomes an
|
||||
**enforced sandbox boundary** in Phase 3.
|
||||
|
||||
| Service | Import | Stability |
|
||||
|---------|--------|-----------|
|
||||
| Open an app window | `openAppWindow(id)` from `$lib/stores/windows` | Stable |
|
||||
| Open an entity window | `openEntityWindow(slug)` from `$lib/stores/windows` | Stable |
|
||||
| Open a task window | `openTaskWindow(sessionId, title)` from `$lib/stores/windows` | Stable |
|
||||
| Dashboard summary | `summary`, `subscribeContext` from `$lib/stores/context` | Stable |
|
||||
| Live events | `subscribeEvents` from `$lib/stores/events` | Stable |
|
||||
| Per-session chat | `chatFor(sessionId)` from `$lib/stores/chat` | Stable |
|
||||
| Per-session workspace | `workspaceFor(sessionId)` from `$lib/stores/workspace` | Stable |
|
||||
| REST API | `$lib/api` functions | Stable (generated from OpenAPI) |
|
||||
| UI primitives | `$lib/components/ui/*` | Stable |
|
||||
| Theme | `getTheme`, `setTheme` from `$lib/stores/theme.svelte` | Stable |
|
||||
|
||||
### 2.5 Content resolution — fixed
|
||||
|
||||
Replace the positional `if/else` chain with a prefix → component map owned
|
||||
by the shell:
|
||||
|
||||
```typescript
|
||||
// WindowLayer.svelte — one map, dispatch by prefix. New window kinds
|
||||
// register here, not in an if/else chain.
|
||||
const CONTENT_RESOLVERS: Array<[prefix: string, resolve: (id: string) => Component | null]> = [
|
||||
['session:', () => SessionChatWindow],
|
||||
['app:', (id) => appById.get(id.slice(4))?.component ?? null],
|
||||
]
|
||||
|
||||
function resolveContent(id: string): Component | null {
|
||||
if (id === NEW_TASK_WINDOW_ID) return NewTaskChat
|
||||
for (const [prefix, resolve] of CONTENT_RESOLVERS) {
|
||||
if (id.startsWith(prefix)) return resolve(id)
|
||||
}
|
||||
return EntityDetailContent // bare entity slug fallback
|
||||
}
|
||||
```
|
||||
|
||||
Adding a `'lxc:'` console window kind later = one array entry. The
|
||||
existing orphan-close effect (`WindowLayer.svelte:25-30`) is kept as-is;
|
||||
Phase 2 must gate it on registry-ready (§5).
|
||||
|
||||
### 2.6 Designed extension points (documented, not built)
|
||||
|
||||
| Extension | Mechanism when built | Trigger |
|
||||
|-----------|---------------------|---------|
|
||||
| Titlebar actions | `titlebarActions?: Component` on AppDef, rendered left of min/max/close | First app that needs one |
|
||||
| App-scoped state | `state?: () => Record<string, unknown>` on AppDef | First app with cross-mount state that isn't module-scoped |
|
||||
| `onRegister` handshake | Called with a scoped AppOS capability object | Phase 3 (dynamic install) |
|
||||
|
||||
Documenting these now prevents the Phase 1 contract from painting itself
|
||||
into a corner; building them now would be speculative.
|
||||
|
||||
---
|
||||
|
||||
## 3. The mascot as an App
|
||||
|
||||
### 3.1 Registration
|
||||
|
||||
```typescript
|
||||
{
|
||||
id: 'mascot',
|
||||
title: 'Cluck',
|
||||
icon: EggIcon, // Lucide egg (chick/adult swap is a future nicety)
|
||||
component: MascotLayer,
|
||||
docked: true,
|
||||
// no width/height — docked
|
||||
// no badge — a permanent "1" is noise, not information
|
||||
}
|
||||
```
|
||||
|
||||
### 3.2 The docked-visibility store (new)
|
||||
|
||||
```typescript
|
||||
// lib/stores/docked.ts
|
||||
// Visibility for docked apps — persisted, so "hidden" survives reloads.
|
||||
// Keyed by app id; absent key = visible (default-on for new docked apps).
|
||||
export const dockedVisibility: Readable<Record<string, boolean>>
|
||||
export function toggleDocked(appId: string): void
|
||||
export function isDockedVisible(appId: string): boolean
|
||||
```
|
||||
|
||||
- localStorage key: `oikos-docked-apps`
|
||||
- Default: visible (a fresh install shows the mascot; hiding is opt-out)
|
||||
- Merge semantics mirror `icons.ts`: unknown persisted keys are kept (an
|
||||
uninstalled docked app that gets reinstalled remembers its state)
|
||||
|
||||
### 3.3 Shell changes
|
||||
|
||||
**`windows.ts` — `openAppWindow` branches on kind:**
|
||||
|
||||
```typescript
|
||||
export function openAppWindow(appId: string): void {
|
||||
const app = appById.get(appId)
|
||||
if (!app) return
|
||||
if (app.docked) { toggleDocked(appId); return } // ← the branch the first draft missed
|
||||
// ... existing wm.open path unchanged
|
||||
}
|
||||
```
|
||||
|
||||
This is the load-bearing detail: the icon click in `Desktop.svelte:93`
|
||||
calls `openAppWindow(app.id)` for every app uniformly. Branching **inside**
|
||||
`openAppWindow` means Desktop.svelte, legacy hash resolution, and any
|
||||
future caller need no special cases.
|
||||
|
||||
**`Desktop.svelte` — replace hardcoded `<MascotLayer />` with:**
|
||||
|
||||
```svelte
|
||||
<DockedLayer />
|
||||
```
|
||||
|
||||
**`components/desktop-shell/DockedLayer.svelte` — new, ~30 lines:**
|
||||
|
||||
```svelte
|
||||
{#each APPS.filter(a => a.docked) as app (app.id)}
|
||||
{#if $dockedVisibility[app.id] ?? true}
|
||||
<app.component />
|
||||
{/if}
|
||||
{/each}
|
||||
```
|
||||
|
||||
Rendered after `<WindowLayer />` inside the surface div, so docked apps
|
||||
share the surface's coordinate space (the mascot's ground-line computation
|
||||
depends on this — `MascotLayer.svelte:9-13`).
|
||||
|
||||
**`MascotLayer.svelte` — zero changes.** No props today, no props after.
|
||||
|
||||
### 3.4 What the mascot gains
|
||||
|
||||
| Feature | Before | After |
|
||||
|---------|--------|-------|
|
||||
| Registry entry | None — hardcoded in shell | First-class AppDef |
|
||||
| Show/hide | Impossible — always mounted | Icon click toggles; persists across reloads |
|
||||
| Shell coupling | `Desktop.svelte` imports mascot internals | Shell knows only `AppDef` |
|
||||
| Precedent for overlay apps | None | Any `docked: true` app (clock, net monitor) uses the same path |
|
||||
|
||||
### 3.5 What the mascot does *not* gain (deliberately)
|
||||
|
||||
- **No taskbar button.** No window → no taskbar entry. The desktop icon is
|
||||
the control.
|
||||
- **No window chrome.** It's a desktop creature, not a document.
|
||||
- **No settings panel in v1.** Hatch/rename/pet/feed stay in the existing
|
||||
radial menu. A mascot *settings* surface (volume, behavior toggles) would
|
||||
be a separate windowed app later — noted as a follow-up idea, not
|
||||
planned.
|
||||
|
||||
### 3.6 UX risk: "where did my chicken go?"
|
||||
|
||||
Hidden state persists across reloads. Mitigation: the desktop icon is
|
||||
always present and is the obvious toggle; the icon's tooltip reads
|
||||
"Cluck — click to show/hide". Acceptable.
|
||||
|
||||
---
|
||||
|
||||
## 4. Current apps — conformance audit
|
||||
|
||||
| App | Conforms? | Notes |
|
||||
|-----|-----------|-------|
|
||||
| **Tasks** (`Overview.svelte`) | ✅ Full | Self-contained. Opens session windows via `openTaskWindow`. |
|
||||
| **Knowledge Base** (`KnowledgeBase.svelte`) | ✅ Full | Opens entity windows via `openEntityWindow`. |
|
||||
| **Operations** (`Ops.svelte`) | ✅ Full | Badge reads `summary`. |
|
||||
| **Signals** (`Signals.svelte`) | ✅ Full | Opens entity windows. |
|
||||
| **Knowledge** (`Knowledge.svelte`) | ✅ Full | — |
|
||||
| **Learning** (`Learning.svelte`) | ✅ Full | — |
|
||||
| **Settings** (`Settings.svelte`) | ✅ Full | Opened from taskbar tray too — same `openAppWindow` path. |
|
||||
| **Mascot** | ❌ Not an App | Hardcoded in Desktop.svelte. Refactored per §3. |
|
||||
|
||||
All seven windowed apps conform today. "Independently shippable" at Phase 1
|
||||
means: add = one page file + one registry entry; remove = delete both. No
|
||||
shell edits, no inter-app imports (apps open each other's surfaces only
|
||||
through AppOS primitives).
|
||||
|
||||
---
|
||||
|
||||
## 5. Extensibility roadmap
|
||||
|
||||
### Phase 1: Strengthen the contract (this plan)
|
||||
|
||||
- [x] `AppDef` extended: `docked`, `noIcon`; geometry conditional on kind
|
||||
- [x] `lib/stores/docked.ts`: docked-visibility store, persisted
|
||||
- [x] `openAppWindow` branches on `docked`
|
||||
- [x] `DockedLayer.svelte`: generic docked-app layer in Desktop.svelte
|
||||
- [x] Mascot registered as `docked: true`; hardcoded `<MascotLayer />` removed
|
||||
- [x] WindowLayer: prefix-map content resolution *(deferred — re-audited as gold-plating; original gate already handles orphans)*
|
||||
- [x] `apps.test.ts`: validation rules per kind (§2.3)
|
||||
- [x] AppOS contract documented (§2.4 lands in MBSE component doc)
|
||||
|
||||
### Phase 2: Lazy loading
|
||||
|
||||
- [x] `component` becomes `() => Promise<{ default: Component }>`; all apps use dynamic imports
|
||||
- [x] Desktop icons render immediately (metadata only); component chunk loads on window open
|
||||
- [x] `LazyApp.svelte` — shared loading skeleton (spinner) used by WindowLayer + DockedLayer
|
||||
- [x] Deleted `LazyMascot.svelte` — the registry lazy loader breaks the cycle directly
|
||||
- [x] Vite code-splits each app into its own chunk (main bundle 800KB → 482KB)
|
||||
- [ ] Icon store revalidates against live registry *(Phase 3 prerequisite — not needed while apps are statically registered)*
|
||||
- [ ] WindowLayer orphan-close gated on registry-ready *(Phase 3 prerequisite)*
|
||||
|
||||
### Phase 3: Dynamic app installation (frontend scaffold, local bundles)
|
||||
|
||||
Scoped at execution time to **local bundles only** (remote-URL loading +
|
||||
sandboxing deferred to Phase 4 — security-critical, needs ADR + careful
|
||||
design). The mechanism built here generalizes to remote bundles by
|
||||
swapping the catalog for a fetched manifest + `import(/* @vite-ignore */ url)`.
|
||||
|
||||
- [x] `AppManifest` format (id, title, permissions, version, geometry) — `web/src/app-store/catalog.ts`
|
||||
- [x] `AppPermission` enum (declaration-only; enforcement is Phase 4)
|
||||
- [x] Static catalog with one demo app (Notes) — `web/src/app-store/apps/Notes.svelte`
|
||||
- [x] Runtime registry: `APPS` → derived store (built-ins + installed); `appById` → derived Map
|
||||
- [x] `installApp` / `uninstallApp` + localStorage persistence (`oikos-installed-apps`)
|
||||
- [x] `icons.ts` reactive to app registration (late-registering apps get free cells; reset re-seeds from live registry)
|
||||
- [x] WindowLayer orphan-close reactive to `$appById` (reinstall revives, uninstall closes)
|
||||
- [x] App Store page (`web/src/pages/AppStore.svelte`) — list / install / uninstall
|
||||
- [x] Installed apps appear on desktop immediately (no reload); uninstall removes icon + closes window
|
||||
- [x] Icon store revalidates against live registry *(the Phase 3 prerequisite — now done)*
|
||||
- [ ] `/api/v1/apps` endpoint + DB-backed manifest storage *(Phase 4)*
|
||||
- [ ] Remote bundle loading from URLs + CSP + capability sandboxing *(Phase 4)*
|
||||
- [ ] Permission enforcement at AppOS boundary *(Phase 4)*
|
||||
|
||||
### Phase 4: Marketplace (vision)
|
||||
|
||||
- [ ] Community apps (network map, backup dashboard, energy monitor)
|
||||
- [ ] Versioning + auto-update
|
||||
- [ ] Mascot skin packs as installable docked-app variants
|
||||
|
||||
---
|
||||
|
||||
## 6. Implementation — Phase 1, file by file
|
||||
|
||||
| # | File | Change |
|
||||
|---|------|--------|
|
||||
| 1 | `lib/apps.ts` | Extend `AppDef` (`docked?`, `noIcon?`, geometry optional). Register mascot. Import `MascotLayer` + `EggIcon`. |
|
||||
| 2 | `lib/stores/docked.ts` | **New.** `dockedVisibility` store, `toggleDocked`, `isDockedVisible`, localStorage persistence. |
|
||||
| 3 | `lib/stores/windows.ts` | `openAppWindow`: docked branch → `toggleDocked`. |
|
||||
| 4 | `components/desktop-shell/DockedLayer.svelte` | **New.** Renders visible docked apps after WindowLayer. |
|
||||
| 5 | `components/desktop-shell/Desktop.svelte` | Replace `import MascotLayer` + `<MascotLayer />` with `<DockedLayer />`. |
|
||||
| 6 | `components/desktop-shell/WindowLayer.svelte` | **Deferred during implementation.** The positional if/else was re-audited and found to already handle orphans cleanly (`{#if win && (!appId || app)}`), and any new window kind needs a prop-dispatch branch in markup regardless — so a prefix→component map adds machinery without decoupling. Documented as an extension point (§2.5) like `titlebarActions`; not built (YAGNI). |
|
||||
| 7 | `lib/apps.test.ts` | Mock `MascotLayer` import (same pattern as pages). Per-kind validation tests. Docked apps exempt from positive-size test. |
|
||||
| 8 | `lib/stores/docked.test.ts` | **New.** Toggle, persistence, default-visible, unknown-key merge. |
|
||||
| 9 | `docs/mbse/components.md` | Add Component 9: Web Control Room — App Architecture (§7). |
|
||||
|
||||
**Out of scope for Phase 1:** `titlebarActions`, app-scoped state,
|
||||
lazy loading, manifests, permissions.
|
||||
|
||||
**Verification:**
|
||||
|
||||
```bash
|
||||
cd web
|
||||
npm run test # vitest — registry + docked store
|
||||
npm run check # svelte-check + tsc
|
||||
npm run lint
|
||||
npm run build # vite build — confirms no import cycles from DockedLayer
|
||||
```
|
||||
|
||||
Manual smoke: icon toggle hides/shows mascot → reload → stays hidden →
|
||||
toggle → returns at last position (model `lastPos` restore). All seven
|
||||
windowed apps open/focus/close identically to before. Legacy hash
|
||||
`#/signals` still opens the Signals window.
|
||||
|
||||
---
|
||||
|
||||
## 7. MBSE documentation
|
||||
|
||||
Add **Component 9: Web Control Room — App Architecture** to
|
||||
`docs/mbse/components.md`:
|
||||
|
||||
```
|
||||
9. Web Control Room — App Architecture
|
||||
9.1 Purpose — OS + Apps metaphor, why apps are independently shippable
|
||||
9.2 Structural View — shell modules, registry, docked layer (mermaid)
|
||||
9.3 App Contract — AppDef, validation rules, app kinds
|
||||
9.4 OS-Service Surface — the AppOS table
|
||||
9.5 Content Resolution — prefix map, window kinds, orphan cleanup
|
||||
9.6 Behavior — window state machine, docked visibility lifecycle
|
||||
9.7 Requirements — WEB-APP-* traceability
|
||||
9.8 Verification — test coverage, manual smoke
|
||||
```
|
||||
|
||||
### Requirements
|
||||
|
||||
| ID | Requirement | Status |
|
||||
|----|-------------|--------|
|
||||
| WEB-APP-1 | Apps register via data-driven AppDef entries; no shell edits to add/remove | ✅ live |
|
||||
| WEB-APP-2 | Apps render in wmkit floating windows | ✅ live |
|
||||
| WEB-APP-3 | Window IDs namespaced (`app:`/`session:`/entity) — no collisions | ✅ live |
|
||||
| WEB-APP-4 | Desktop icons render from the registry | ✅ live |
|
||||
| WEB-APP-5 | Taskbar buttons derive from window state, icons resolved via registry | ✅ live |
|
||||
| WEB-APP-6 | Removed apps' persisted windows self-close | ✅ live (`WindowLayer.svelte:25-30`) |
|
||||
| WEB-APP-7 | Content resolution dispatches via prefix map, not positional if/else | ⬜ Deferred — re-audited; original gate already handles orphans, map adds no decoupling (§2.5) |
|
||||
| WEB-APP-8 | Docked app kind: no window, no chrome, visibility toggled via icon | ⬜ Phase 1 |
|
||||
| WEB-APP-9 | Mascot is a registered docked App, not a hardcoded shell component | ⬜ Phase 1 |
|
||||
| WEB-APP-10 | Docked visibility persists across reloads | ⬜ Phase 1 |
|
||||
| WEB-APP-11 | OS-service surface (AppOS) documented as the stable App API | ⬜ Phase 1 |
|
||||
| WEB-APP-12 | Registry validation: per-kind geometry rules enforced by tests | ⬜ Phase 1 |
|
||||
| WEB-APP-13 | Apps lazy-load; icons render from static metadata | ✅ Phase 2 |
|
||||
| WEB-APP-14 | Icon store revalidates against live registry, not import-time snapshot | ✅ Phase 3 |
|
||||
| WEB-APP-15 | Third-party apps install from manifests with declared permissions | ✅ Phase 3 (local bundles; enforcement Phase 4) |
|
||||
|
||||
### Sequence — windowed app open
|
||||
|
||||
```mermaid
|
||||
sequenceDiagram
|
||||
participant User
|
||||
participant Desktop
|
||||
participant WM as Window Manager
|
||||
participant WL as Window Layer
|
||||
participant App
|
||||
|
||||
User->>Desktop: click icon
|
||||
Desktop->>WM: openAppWindow("signals")
|
||||
Note over WM: docked? no → wm path
|
||||
alt window exists
|
||||
WM->>WM: restore + focus
|
||||
else new
|
||||
WM->>WM: wm.open({ id: "app:signals", ... })
|
||||
WM->>WL: render frame
|
||||
WL->>WL: resolveContent → prefix 'app:' → registry
|
||||
WL->>App: mount component
|
||||
end
|
||||
WM->>Taskbar: new button in wmState.order
|
||||
```
|
||||
|
||||
### Sequence — docked app toggle
|
||||
|
||||
```mermaid
|
||||
sequenceDiagram
|
||||
participant User
|
||||
participant Desktop
|
||||
participant Dock as docked.ts
|
||||
participant Layer as DockedLayer
|
||||
participant App
|
||||
|
||||
User->>Desktop: click Cluck icon
|
||||
Desktop->>Dock: openAppWindow("mascot") → docked → toggleDocked
|
||||
Dock->>Dock: flip visibility, persist localStorage
|
||||
Dock->>Layer: store update
|
||||
alt now visible
|
||||
Layer->>App: mount MascotLayer
|
||||
Note over App: model + sprites restore<br/>from module scope
|
||||
else now hidden
|
||||
Layer->>App: unmount (state survives)
|
||||
end
|
||||
```
|
||||
|
||||
### State machine — app window
|
||||
|
||||
```mermaid
|
||||
stateDiagram-v2
|
||||
[*] --> Closed: registered, no window
|
||||
Closed --> Open: openAppWindow
|
||||
Open --> Focused: focus
|
||||
Focused --> Open: blur
|
||||
Open --> Minimized: minimize
|
||||
Minimized --> Focused: restore
|
||||
Open --> Closed: close
|
||||
Minimized --> Closed: close
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 8. Risk & safety
|
||||
|
||||
| Risk | Severity | Mitigation |
|
||||
|------|----------|------------|
|
||||
| Mascot refactor breaks stimuli or ground-line computation | Medium | MascotLayer unchanged; DockedLayer mounts it in the same surface div, same position in the stacking order as today. |
|
||||
| Hidden mascot never rediscovered | Low | Icon always present, tooltip says show/hide. |
|
||||
| `openAppWindow` docked branch leaks into windowed path | Low | Branch is the first statement; windowed path byte-identical. Covered by existing call sites (icon click, taskbar settings, legacy hash). |
|
||||
| Docked visibility store desyncs from registry | Low | Unknown keys kept on load; layer filters by `a.docked` from the live registry. |
|
||||
| Phase 2 lazy loading kills persisted windows of not-yet-loaded apps | Medium | Explicit Phase 2 gate: orphan-close waits for registry-ready (§5). Called out now so it isn't discovered in production. |
|
||||
|
||||
---
|
||||
|
||||
## 9. Appendix: relevant existing artifacts
|
||||
|
||||
| Artifact | Relevance |
|
||||
|----------|-----------|
|
||||
| `docs/mbse/README.md` §5 | MCP tools / REST / SSE — the data surface Apps consume |
|
||||
| `docs/mbse/components.md` §5 | Current web control room component doc — Phase 1 extends it |
|
||||
| `docs/mascot/README.md` | Mascot subsystem model (MASC-1..12); MASC-9's registry philosophy is the template for this plan |
|
||||
| `plans/2026-07-08-control-room-webui.md` | Original control-room plan |
|
||||
| `plans/done/2026-07-11-ui-review-ia-usability.md` | IA review that produced the desktop metaphor |
|
||||
| `plans/2026-07-20-desktop-mascot.md` | Mascot plan; extension registries |
|
||||
| `lib/apps.ts` header comment | Already documents the one-entry-to-add-an-app philosophy |
|
||||
|
||||
---
|
||||
|
||||
*Plan opened 2026-07-21. Phase 1 ready for execution — estimated small
|
||||
(~half a day of focused work; nine file touches, two new files). Phases
|
||||
2–4 are context for future sessions and do not block Phase 1.*
|
||||
@@ -0,0 +1,226 @@
|
||||
# 2026-08-03 — Session review: `service:haos` monitoring + agent capability gaps
|
||||
|
||||
**Reviewed 2026-08-16 — Status: Won't do** — agent capability gaps plan superseded by hexagonal refactor; re-evaluation needed in new architecture.
|
||||
|
||||
**Status:** Plan (audit complete; ready to implement).
|
||||
**Reviewed session:** `23da10db-46a9-444c-bbde-ca9457bd9087` — *"Work out what
|
||||
monitoring checks service:haos should have and configure them."*
|
||||
**Method:** Direct Postgres read of `agent_sessions`/`agent_messages`/
|
||||
`agent_activity`/`session_plan_steps` on the prod mac-mini (oikos prod runs here
|
||||
in docker compose project `oikos`; gateway `:8092`), cross-referenced with the
|
||||
code paths in `internal/mcp`, `internal/httpapi`, `internal/policy`,
|
||||
`internal/checkdefaults`, `internal/db/seed.go`.
|
||||
|
||||
---
|
||||
|
||||
## 1. Session audit (objective vs outcome)
|
||||
|
||||
| Dimension | Finding |
|
||||
|---|---|
|
||||
| Objective | Determine + configure monitoring checks for `service:haos` (HAOS VM 108, `home.hubris.network`, `192.168.8.101:8123`). |
|
||||
| Outcome | ❌ **Failed/stuck.** `status=executing`, `outcome=null` ~4 min after last activity (UTC); never reached a terminal state. Only the *existing* `check:vm-status:vm:haos:0` stub got populated; the three **new** checks (`http:service`, `http:ingress`, `cert-expiry`) and their `ingress:`/`cert:` entities were never created. |
|
||||
| Tool calls | **116** (vs the >30 N+1 failure signature). ~45 redundant `list_entities`/`get_entity`/`get_relations`, then a ~15-min storm of `run` doing `find`/`grep`/`cat` on prod source. |
|
||||
| Plan | 8 steps proposed; steps 1–4 genuinely done; **step 5 falsely marked "done"** after both its tool calls errored `entity not found`; steps 6–8 never started. |
|
||||
| Operator friction | 3 manual interventions: `status`, `proceed`, *"why dony you use the mcp?"*; plus a **44-minute approval stall** (19:52→20:36) on two trivial reachability curls. |
|
||||
| Severity | **blocker** (capability gap) + **friction** (classifier, plan-state, reaping). |
|
||||
|
||||
### Timeline (UTC)
|
||||
- **19:45–19:50** — read-only exploration; `run` correctly blocked ("No plan… call set_goal then propose_plan"). Good guard.
|
||||
- **19:50** — `propose_plan` (8 steps).
|
||||
- **19:52** — two `curl … -o /dev/null -w '%{http_code}'` reachability probes → both classified `config_mutation` → one queued for approval (`019fc92e…`), second blocked ("approval already pending").
|
||||
- **19:52 → 20:36 (44 min)** — idle, waiting on operator approval.
|
||||
- **20:36** — approval granted ("auto via assent window"); both curls → 200/200.
|
||||
- **20:37** — step 4 ✅: populated `check:vm-status:vm:haos:0` + `checks` edge.
|
||||
- **20:37:58** — step 5 ❌: `update_entity_attributes("check:http:service:haos:0")` → **`entity not found`**; `create_relationship` → **`source entity not found`**. *(There is no create tool.)*
|
||||
- **20:38–20:47** — spiral: `search_knowledge` (empty), then `run find/grep/cat` across `/opt/homelab-context/**/*.go` to reverse-engineer check creation. Reads `checkdefaults.go`, `monitoring.go`, `default_checks.go`, `checks.go`, `coverage.go`.
|
||||
- **20:42** — sets `service:haos` `monitoring: ["http"]` via `update_entity_attributes`, hoping `checkdefaults.Ensure()` auto-generates. **It does not** (see A2).
|
||||
- **20:42–20:50** — tries to reach the REST API directly: `psql` on hubris (cmd 127), `docker exec` on hubris (docker absent), `curl http://192.168.178.25:8090` (wrong subnet; real net is `192.168.8.x`; exit 7), `curl http://oikos-api:8090` (MCP routes to hubris which can't resolve the mac-mini docker alias; 30s timeouts ×2), `ssh root@192.168.178.25` (no route). Final `update_entity_attributes` on `ingress:`/`cert:` → `not found`.
|
||||
- **20:50:38** — last activity: a failed 30s `run`. Session goes silent, never terminates.
|
||||
|
||||
---
|
||||
|
||||
## 2. Root-cause findings (with code evidence)
|
||||
|
||||
### A1 — No entity-creation capability in the MCP toolset *(the blocker)*
|
||||
`internal/mcp/tools.go` registers **37 tools**; the only entity-mutation surface is
|
||||
`update_entity_attributes` (merge into an **existing** entity) and
|
||||
`create_relationship` (needs **existing** source+target). Neither can create a new
|
||||
entity. The capability **does** exist at the HTTP layer — `CreateEntity`
|
||||
(`internal/httpapi/impl.go:865`, `POST /api/v1/entities`) — it is simply not exposed
|
||||
to the agent. Every "set up / onboard / configure entity X" task that needs a new
|
||||
check/ingress/cert/service hits this wall.
|
||||
|
||||
### A2 — MCP `update_entity_attributes` bypasses `ensureDefaultChecks`
|
||||
`ensureDefaultChecks` (`internal/httpapi/default_checks.go:20`) is invoked **only**
|
||||
from the HTTP handlers: `CreateEntity` (`impl.go:1012`) and `PatchEntity`
|
||||
(`impl.go:1280`). `grep ensureDefaultChecks internal/mcp/` → **no matches**: the MCP
|
||||
tool writes attributes straight to the store, so flipping `service:haos`
|
||||
`monitoring:["http"]` never regenerated its checks. The agent's fallback strategy
|
||||
was structurally doomed via MCP.
|
||||
|
||||
### A3 — `-o /dev/null` curl idiom misclassified as `config_mutation`
|
||||
`internal/policy/command.go:106` `curlMutateRe` matches `(?:^|\s)-(?:d|F|T|o)\b` — so
|
||||
`-o` (output-file) is treated as mutation. The canonical read-only reachability probe
|
||||
`curl -sS -o /dev/null -w '%{http_code}' …` therefore escalates to approval. This is
|
||||
the entire 44-minute stall. (`curlIsReadOnly` at `command.go:307` only passes for GET
|
||||
with no `-o`/`-d`/`-X`/`>`.) A pure GET that discards the body is the single most
|
||||
common health probe and shouldn't need approval.
|
||||
|
||||
### A4 — No platform self-knowledge doc for the check lifecycle
|
||||
`search_knowledge("create check entity how to add new check monitoring")` → empty.
|
||||
The agent re-derived the whole mechanism from source on prod (~15 min, dozens of
|
||||
`run`). There is no agent/operator runbook explaining: check slugs are
|
||||
`check:<kind>:<target>:<n>`; `check_defs` are derived from the type's `monitoring`
|
||||
spec by `checkdefaults.Ensure`; Ensure runs at **seed/deploy** and on **HTTP
|
||||
create/patch**, not via MCP.
|
||||
|
||||
### A5 — False plan progress (step marked done on failure)
|
||||
At 20:37:58 both tool calls for step 5 returned `error: entity not found`, yet the
|
||||
agent advanced step 5→`done`. Plan-state integrity hole: a step whose actions error
|
||||
should not transition to `done`. (`session_plan_steps` confirms seq 5 = `done`.)
|
||||
|
||||
### A6 — No "missing-capability" escalation; self-grounding failures
|
||||
On detecting the dead-end (no create tool) the agent never told the operator *"I lack
|
||||
a tool to create entities — please create them"*; instead it tried to bypass its own
|
||||
platform. Grounding errors: invented IP `192.168.178.25` (real LAN is `192.168.8.x`),
|
||||
ran `run` against `ws:mac-mini` ("unsupported target — must be host:/lxc:/vm:"),
|
||||
assumed `docker` exists on hubris, assumed the docker-alias `oikos-api` resolves from
|
||||
hubris. The agent didn't query `get_entity("service:oikos")` for the real address.
|
||||
|
||||
### A7 — Sessions never reap from `executing`
|
||||
Last activity 20:50; status still `executing` with no turn running. There is no
|
||||
idle-timeout / abandoned transition when a turn ends without resolution. (Fleet-wide:
|
||||
176 done / 9 failed / 1 executing; the 9 prior failures are pre-v0.15.0, mostly
|
||||
approval-stalls and entity-not-found — same families.)
|
||||
|
||||
### A8 — N+1 tool fan-out (116 calls)
|
||||
Dozens of redundant `list_entities`/`get_entity`/`get_relations` before proposing a
|
||||
plan, plus the source-reading `run` storm. Above the >30-per-turn signature; indicates
|
||||
weak bulk-tool use and under-constrained exploration before planning.
|
||||
|
||||
---
|
||||
|
||||
## 3. Improvement plan (ordered)
|
||||
|
||||
**Scope decision (confirmed with operator):** general `create_entity` MCP tool **+
|
||||
wire regen** — solves this case and the 67-entity blast radius (§4).
|
||||
|
||||
### Task 1 — `create_entity` MCP tool *(fixes A1; the centerpiece)*
|
||||
- Register a new tool `create_entity(slug, type, name, attributes?)` in
|
||||
`internal/mcp/tools.go` that **reuses** `httpapi.CreateEntity`
|
||||
(`impl.go:865`) / the same store path — do not hand-roll. It must run
|
||||
`ensureDefaultChecks` (free, since it goes through the create path).
|
||||
- **Approval policy:** no approval required for the entity itself — it mutates the
|
||||
knowledge graph, matching the existing no-approval stance of
|
||||
`update_entity_attributes`/`create_relationship`/`upsert_knowledge`. (Derived checks
|
||||
are safe/read-side; if a check kind is ever deemed mutating, gate *that* in the
|
||||
scheduler, not here.)
|
||||
- Validate `type` against `entity_types`; reject unknown slugs/types with a clear
|
||||
error. Idempotent on existing slug (return the existing entity, mirroring the HTTP
|
||||
`ETag`/conflict behavior).
|
||||
- Expose to the agent via the tool-list build path used by `cmd/nomos/agent.go`.
|
||||
|
||||
### Task 2 — MCP `update_entity_attributes` triggers `ensureDefaultChecks` *(fixes A2)*
|
||||
- After the attribute merge in the MCP handler, call `ensureDefaultChecks` with the
|
||||
post-merge entity (same args as `impl.go:1280`). This makes "set monitoring → checks
|
||||
regenerate" work via MCP, matching HTTP semantics.
|
||||
- Mind the `default_checks.go:14-19` caveat: a service whose address comes from its
|
||||
host edge may still produce no checks until the hosting edge exists — log/return
|
||||
that as an explicit result so the agent knows to create the edge next.
|
||||
|
||||
### Task 3 — Classifier: read-only `curl` with `-o /dev/null` *(fixes A3)*
|
||||
- In `internal/policy/command.go` `curlIsReadOnly`, treat `-o /dev/null` (and
|
||||
`--output /dev/null`) as read-only — it's a no-op sink. Keep `-o <realpath>` as
|
||||
mutation. Add `TestClassifyCommand_CurlDevNull_ReadOnly` next to the existing
|
||||
`TestClassifyCommand_CurlPipeSh_ConfigMutation`.
|
||||
- Coach complement: in `nomos/SOUL.md`, note that reachability probes should use
|
||||
`curl -I` or `-o /dev/null` GETs (now read-only) rather than POSTs.
|
||||
|
||||
### Task 4 — Plan-state integrity: don't mark `done` on errored actions *(fixes A5)*
|
||||
- In `cmd/nomos` (`agent.go`/`tasks.go` where `update_plan_step` is emitted), a step
|
||||
whose turn ended with only error/`not-found` tool results must **not** auto-advance
|
||||
to `done`; leave it `running`/`blocked` and surface the failure to the operator.
|
||||
Minimal: if every tool call in the step returned an `error:*` result, hold the step.
|
||||
|
||||
### Task 5 — Stuck-session reaping *(fixes A7)*
|
||||
- Add an idle sweep (extend the existing continuation/idle worker in `cmd/nomos`) that
|
||||
transitions a session from `executing`→`failed` (or a new `stuck`) when no turn has
|
||||
run for N minutes and no approval is pending. Emit an event so the UI (F3 terminal
|
||||
handling) clears the spinner. Pick N (recommend 30 min) — confirm in review.
|
||||
|
||||
### Task 6 — Missing-capability escalation + grounding *(fixes A6)*
|
||||
- `nomos/SOUL.md`: when a mutation tool returns `entity … not found` on a create
|
||||
intent, the agent must **stop and ask the operator** (or now use `create_entity`)
|
||||
rather than pivot to `run`/SSH/API-bypass. Forbidden: inventing IPs/subnets; instead
|
||||
`get_entity("service:oikos")` for the real API address. `run` targets must be
|
||||
`host:/lxc:/vm:` slugs (state the contract explicitly).
|
||||
|
||||
### Task 7 — Runbook: "how checks work / how to add monitoring" *(fixes A4)*
|
||||
- Upsert a knowledge doc (via `upsert_knowledge`, linked to the `agent:nomos` and
|
||||
`document:infrastructure/monitoring` entities) covering: check slug grammar,
|
||||
`checkdefaults.Ensure` triggers (seed + HTTP create/patch, now also MCP), the
|
||||
`monitoring` per-entity override, the host-edge caveat, and the canonical way to add
|
||||
monitoring to an entity (create/patch entity → checks derive).
|
||||
|
||||
### Task 8 — (Lower priority) exploration budget / bulk-tool use *(A8)*
|
||||
- `nomos/SOUL.md`: prefer `list_entities(limit)` + `get_entity_knowledge` bulk calls
|
||||
over N+1 `get_entity`/`get_relations` fans; cap pre-plan exploration. Optional
|
||||
guardrail in `agent.go` (warn at >N same-tool calls per turn).
|
||||
|
||||
### Recommended sequence
|
||||
1 → 2 → 3 → 4 → 7 → 5 → 6 → 8. (1+2 unblock the whole task class; 3 kills the
|
||||
approval stall; 4+5 fix state integrity; 7 is cheap leverage; 6+8 are persona
|
||||
hardening.)
|
||||
|
||||
---
|
||||
|
||||
## 4. Uncovered cases — the capability-gap blast radius
|
||||
|
||||
The existing F1–F8 plans (`2026-08-03-nomos-chat-reliability-and-ux-audit.md`,
|
||||
shipped v0.15.0) and the turn-scheduler review cover **only** UI / streaming / turn
|
||||
serialization / connection UX. **None** addresses agent *capability* or
|
||||
MCP↔HTTP integration. This session exposes the uncovered class:
|
||||
|
||||
- **67 entities currently have no `check:` relationship** (DB query): 40 `lxc`, 26
|
||||
`service`, 1 `vm`. Any "add monitoring to X" task fails identically until Tasks 1+2.
|
||||
- **Whole task families blocked by the no-create gap:** onboarding a new host/LXC/VM,
|
||||
declaring a new service/ingress/cert/dns, adding any check that doesn't already
|
||||
exist, registering a relationship target that doesn't exist yet. All currently
|
||||
require an operator to hand-edit `seeds/inventory.yaml` and re-seed.
|
||||
- **MCP↔HTTP semantic drift (generalize A2):** audit other MCP mutation tools for
|
||||
side-effects that the HTTP handlers perform but the MCP path skips (check regen,
|
||||
drift-flagging, audit fields, idempotency). Each is a latent "agent did the right
|
||||
thing but nothing happened" bug.
|
||||
- **Classifier read-only false-positives (generalize A3):** beyond `-o /dev/null`,
|
||||
review other common read-only idioms that escalate (`curl` with benign flags,
|
||||
compound read-only commands) — friction compounds into approval stalls and stuck
|
||||
sessions.
|
||||
- **No terminal/`stuck` reaping (generalize A7):** any turn that ends unresolved
|
||||
leaves the session `executing` forever; the UI never shows "done/failed".
|
||||
|
||||
---
|
||||
|
||||
## 5. Validation
|
||||
|
||||
- **Task 1/2:** `go test ./internal/mcp/... ./internal/httpapi/...` — new test creates
|
||||
`check:http:service:haos:0` via `create_entity`, asserts the entity exists **and**
|
||||
that a `check_def` row was derived; then `update_entity_attributes(service:haos,
|
||||
monitoring:["http"])` via MCP and assert checks regenerate (currently absent).
|
||||
- **Task 3:** `go test ./internal/policy/` — `curl -sS -o /dev/null -w '%{http_code}'
|
||||
URL` ⇒ `read_only`; `curl -o /tmp/x URL` ⇒ `config_mutation`.
|
||||
- **Task 4:** `cmd/nomos` test — a step whose only tool result is `error:*` stays
|
||||
non-`done`.
|
||||
- **Task 5:** idle-sweep test — session with no turn for N min and no pending approval
|
||||
⇒ `failed` (+ event emitted).
|
||||
- **End-to-end re-run:** replay the haos goal against a local nomos; expect the three
|
||||
checks + `ingress:`/`cert:` entities created in <15 tool calls with **zero**
|
||||
approvals and a `done` outcome.
|
||||
|
||||
## 6. Out of scope / open questions
|
||||
- Whether `create_entity` for sensitive types (e.g. `secret`, `key`) should require
|
||||
approval even though it's graph-only — recommend: same no-approval stance now, add
|
||||
type-specific gating later if abused.
|
||||
- The exact stuck-reap window N (recommend 30 min) and whether to introduce a distinct
|
||||
`stuck` status vs reuse `failed`.
|
||||
- Whether to also expose a `delete_entity`/`retire_entity` MCP tool (not needed for
|
||||
this case; lifecycle retirement is a separate flow).
|
||||
564
plans/done/2026-08-05-backend-evaluation-improvements.md
Normal file
564
plans/done/2026-08-05-backend-evaluation-improvements.md
Normal file
@@ -0,0 +1,564 @@
|
||||
# 2026-08-05 — Backend evaluation: architecture, security, and reliability improvements
|
||||
|
||||
**Reviewed 2026-08-16 — Status: Completed** — Phases 0, 2, 3, 4, 6 implemented and deployed (0.28.0–0.29.0); Phase 1 security and Phase 5 observability remain backlog.
|
||||
|
||||
Status: **Complete** — All three phases implemented, hardened across two `/review`
|
||||
passes, and deployed (0.28.0–0.29.0, Aug 8 2026).
|
||||
|
||||
- **Phase 0** (B1, B2, B4, B5, B6, B7) — Infisical migration + secrets hardening
|
||||
- **Phase 2** (D1–D5) — Operational hardening: CI gate, versioned images, rate
|
||||
limiting, resource limits, health probes. Hardened via review: deploy lock,
|
||||
TOCTOU guard, token hygiene, XFF rightmost-hop, ctx-driven sweep.
|
||||
- **Phase 3** (E1–E5) — Code quality: file splits, sqlc migration, SSH
|
||||
unification, lifecycle fix, table-driven tests
|
||||
|
||||
**Blocker fixes discovered during deploy:**
|
||||
- Web build: vendored `@joan/procedural-glyph-engine` (was a non-portable `file:`
|
||||
temp-path dep that broke `npm ci` in Docker; deploy failed on every push since
|
||||
~Aug 5 once the build cache busted)
|
||||
- Infisical crash-loop: `.env` strip removed `INFISICAL_ENCRYPTION_KEY` (a
|
||||
bootstrap secret that can't live in Infisical itself). Restored from worktree
|
||||
`.env` backup. JWT secrets are dev defaults (OK — only affects web-UI auth).
|
||||
- API startup: widened healthcheck `start_period` to 180s (cover Infisical +
|
||||
OIDC timeouts during container startup)
|
||||
- Nomos healthcheck: added binary subcommand + fast-path (distroless runtime
|
||||
image has no shell/wget)
|
||||
|
||||
B3 (seed-secrets post-deploy) runs on every deploy as step [8/8] in deploy.sh.
|
||||
Remaining: Phase 1 security (C1–C3) and Phase 4–6 backlog.
|
||||
|
||||
Scope: full evaluation of the oikos backend (Go binaries `oikos`, `nomos`, `webhook`,
|
||||
Postgres/TimescaleDB, Docker deployment, MCP server) excluding frontend clients
|
||||
(`web/` SPA and `desktop/` Wails app). Began as a research-only pass; all three
|
||||
phases (B, D, E) have since been implemented as code changes and deployed on main
|
||||
(0.28.0–0.29.0).
|
||||
|
||||
Method: four parallel research passes (Go backend structure, database schema,
|
||||
deployment/infrastructure, API/MCP design) plus Infisical secrets audit and
|
||||
dependency analysis of `go.mod`.
|
||||
|
||||
---
|
||||
|
||||
## A. Summary of findings
|
||||
|
||||
The backend is well-architected with strong fundamentals: contract-first API
|
||||
(oapi-codegen), type-safe SQL (sqlc + pgx), TimescaleDB observability, policy-
|
||||
governed autonomy, and a sophisticated ontology-driven data model. The main gaps
|
||||
are secret management (Infisical is wired but barely used — 3 of 4 binaries
|
||||
read secrets from env/plaintext), operational maturity (CI, image tagging, backup
|
||||
reliability), security hardening (SSH host keys, unauthenticated endpoints), and
|
||||
code hygiene (monolithic files, mixed SQL access patterns).
|
||||
|
||||
| Category | Grade | Notes |
|
||||
|----------|-------|-------|
|
||||
| Tech stack | A | Go + pgx + sqlc + TimescaleDB + chi + slog — all correct choices |
|
||||
| Data model | A- | Dual-entity pattern, temporal relationships, partial indexes, 6 state machines |
|
||||
| API design | B+ | Contract-first with ~50 MCP tools, RFC 9457 errors; no rate limiting |
|
||||
| Secret management | **D** | Infisical SDK wired but only in API/MCP tools path; nomos, webhook, scheduler, notifier all read plaintext env vars. 7 production secrets in `.env`, HMAC in world-readable plist. SOPS fallback is dead code. |
|
||||
| Security | C | OIDC+static token auth is good, but SSH host keys disabled, unauthenticated nomos endpoint, HMAC secret in world-readable plist |
|
||||
| Deployment | C+ | Multi-stage builds, pre-deploy backups, but no CI, no versioned images, silent backup failures |
|
||||
| Code quality | B | Good error handling, panic safety, doc; but 3 files over 1100 lines, mixed raw/sqlc SQL |
|
||||
| Performance | B | Appropriate for scale; SSH check storm risk, no query caching |
|
||||
| Observability | B- | TimescaleDB hypertables + SSE + slog; no Prometheus/Grafana, no OTel tracing |
|
||||
| Testing | C | `make test` exists but many core packages (scheduler, domain, actuator, policy) have 0% coverage |
|
||||
|
||||
## B. Infisical consolidation (critical)
|
||||
|
||||
Infisical is deployed (Redis + Infisical service in compose, Go SDK in go.mod,
|
||||
`internal/secrets/infisical.go` fully implemented) but severely underutilized.
|
||||
Only the API server's MCP tools path creates an Infisical backend. Every other
|
||||
binary reads secrets from env vars or plaintext files.
|
||||
|
||||
### Current wiring map
|
||||
|
||||
| Binary / role | Uses Infisical? | Secrets read from env/plaintext |
|
||||
|---------------|----------------|-------------------------------|
|
||||
| oikos `api` role | Yes (MCP tools only) | `OIKOS_DATABASE_URL`, `OIKOS_MCP_BEARER_TOKEN`, `OIKOS_API_TOKEN`, `OIKOS_OIDC_CLIENT_SECRET` |
|
||||
| oikos `scheduler` role | **No** | `OIKOS_SSH_KEY_PATH`, `OIKOS_DATABASE_URL` |
|
||||
| oikos `notifier` role | **No** | `OIKOS_MATRIX_TOKEN`, `OIKOS_APPROVAL_HMAC_SECRET`, `OIKOS_DATABASE_URL` |
|
||||
| nomos | **No** | `OPENROUTER_API_KEY`, `OIKOS_MCP_BEARER_TOKEN`, `DATABASE_URL` |
|
||||
| webhook | **No** | `WEBHOOK_HMAC_SECRET` (also hardcoded in plist) |
|
||||
|
||||
### B1. Wire Infisical into all binaries at startup
|
||||
|
||||
- **Goal**: Every binary fetches its secrets from Infisical at startup instead of
|
||||
relying on env vars. Bootstrap-only env vars (`INFISICAL_CLIENT_ID`,
|
||||
`INFISICAL_CLIENT_SECRET`, `INFISICAL_SITE_URL`, `INFISICAL_PROJECT_ID`,
|
||||
`OIKOS_DATABASE_URL`) remain as env vars (chicken-egg).
|
||||
- **Approach**: Add a `secrets.InitFromEnv(ctx)` call to each binary's `main()` that
|
||||
creates a `secrets.Manager` (primary Infisical + SOPS fallback). Store the
|
||||
manager in a package-level var or pass it through the initialization chain.
|
||||
- **Files to change**:
|
||||
- `cmd/oikos/main.go` — create Manager in `runWithPool`, pass to scheduler and
|
||||
notifier runners alongside cfg and pool
|
||||
- `cmd/nomos/main.go` — create Manager at startup, fetch `OPENROUTER_API_KEY`
|
||||
and `OIKOS_MCP_BEARER_TOKEN` from Infisical before creating the agent
|
||||
- `cmd/webhook/main.go` — create Manager at startup, fetch `WEBHOOK_HMAC_SECRET`
|
||||
from Infisical
|
||||
- **Secrets to migrate into Infisical** (move from env vars / `.env` / plist):
|
||||
|
||||
| Secret key (in Infisical) | Current source | Used by |
|
||||
|---------------------------|---------------|---------|
|
||||
| `matrix/token` | `OIKOS_MATRIX_TOKEN` env | oikos notifier |
|
||||
| `approval/hmac-secret` | `OIKOS_APPROVAL_HMAC_SECRET` env, plist | oikos notifier, webhook |
|
||||
| `mcp/bearer-token` | `OIKOS_MCP_BEARER_TOKEN` env | oikos api, nomos |
|
||||
| `api/token` | `OIKOS_API_TOKEN` env | oikos api |
|
||||
| `oidc/client-secret` | `OIKOS_OIDC_CLIENT_SECRET` env | oikos api |
|
||||
| `openrouter/api-key` | `OPENROUTER_API_KEY` env | nomos |
|
||||
| `webhook/hmac-secret` | `WEBHOOK_HMAC_SECRET` env + plist | webhook |
|
||||
- **Risk class**: config_mutation
|
||||
- **Prerequisite**: Populate Infisical with these secrets via `oikos secret set` before
|
||||
deploying the code change. Existing `.env` values serve as the source of truth
|
||||
for the initial migration.
|
||||
|
||||
### B2. Activate the SOPS fallback path
|
||||
|
||||
- **Status**: Done (commit pending)
|
||||
- **Current**: `secrets.NewManager(infisical, sops)` is only used in tests.
|
||||
`httpapi/server.go` creates `InfisicalBackend` directly — if Infisical is down,
|
||||
there is no fallback.
|
||||
- **Fix**: Use `secrets.NewManager()` in production everywhere so the SOPS DR
|
||||
fallback actually works when Infisical is unreachable. The Manager's cache
|
||||
(5min TTL) already masks transient Infisical blips.
|
||||
- **What changed**: `httpapi/server.go` now creates `secrets.NewManager(infisical,
|
||||
sopsFallback)` instead of bare `secrets.NewInfisicalBackend`. SOPS backend
|
||||
is created from `cfg.SecretsDir` when set.
|
||||
- **Risk class**: config_mutation
|
||||
|
||||
### B3. Remove `.env` plaintext secrets after migration
|
||||
|
||||
- **Current**: `.env` contains 7 production secrets in plaintext on mac-mini disk.
|
||||
- **Fix**: After B1 is deployed and all binaries read from Infisical, strip
|
||||
secrets from `.env` leaving only non-secret config (`OIKOS_API_LISTEN`,
|
||||
`OIKOS_SCHEDULER_INTERVAL`, etc.). Bootstrap env vars
|
||||
(`OIKOS_DATABASE_URL`, `OIKOS_INFISICAL_*`) stay — they're the trust anchor.
|
||||
- **Risk class**: config_mutation
|
||||
|
||||
### B4. Remove HMAC secret from plist
|
||||
|
||||
- **Status**: Done (code change; plist cleanup is post-deploy)
|
||||
- **Current**: `scripts/deploy/network.hubris.oikos-deploy-webhook.plist` line 14
|
||||
has `WEBHOOK_HMAC_SECRET` hardcoded in plaintext. World-readable.
|
||||
- **Fix**: After B1 (webhook reads from Infisical), remove the `EnvironmentVariables`
|
||||
`WEBHOOK_HMAC_SECRET` entry from the plist entirely. The webhook binary will
|
||||
fetch it from Infisical at startup.
|
||||
- **Risk class**: config_mutation
|
||||
- **Supersedes**: Original plan item B3 (HMAC secret in plist) — same issue,
|
||||
now resolved via Infisical instead of file permissions workarounds.
|
||||
|
||||
### B5. Store SSH host keys in Infisical
|
||||
|
||||
- **Status**: Done (commit pending)
|
||||
- **Current**: `ssh.InsecureIgnoreHostKey()` in 3 code paths
|
||||
(`internal/actuator/ssh.go:160`, `internal/actuator/actuator.go:427`,
|
||||
`internal/mcp/server.go`).
|
||||
- **Fix**: Store Proxmox host public keys in Infisical under
|
||||
`ssh/host-keys/{hostname}`. Actuator reads them at connection init and builds
|
||||
a `knownhosts` callback. For dynamic targets, implement TOFU (trust-on-first-
|
||||
use) writing back to Infisical.
|
||||
- **What changed**:
|
||||
- `internal/actuator/hostkeys.go` — `HostKeyCallback()` returns an
|
||||
`ssh.HostKeyCallback` that verifies against cached keys (MITM detection)
|
||||
and accepts unknown hosts via TOFU, persisting new keys to Infisical.
|
||||
- `internal/actuator/hostkeys_infisical.go` — `InfisicalHostKeySource`
|
||||
implements `HostKeySource` over `secrets.Backend`; `ResolveSSHHosts()`
|
||||
queries DB for active proxmox-host/standalone-server slugs.
|
||||
- `internal/actuator/ssh.go:160` — replaced `InsecureIgnoreHostKey()` with
|
||||
`HostKeyCallback()`.
|
||||
- `internal/actuator/actuator.go:427` — replaced `InsecureIgnoreHostKey()` with
|
||||
`HostKeyCallback()`.
|
||||
- `internal/mcp/server.go:494` — replaced `InsecureIgnoreHostKey()` with
|
||||
`actuator.HostKeyCallback()`.
|
||||
- `internal/httpapi/server.go` — pre-loads SSH host keys from Infisical at
|
||||
startup (queries active hosts, loads their keys from Infisical).
|
||||
- **Post-deploy step**: On first deploy, TOFU will accept all current host
|
||||
keys and store them in Infisical under `ssh/host-keys/{slug}`. Verify the
|
||||
stored keys are correct by checking `oikos secret list`. To pre-populate
|
||||
without TOFU, SSH to each Proxmox host and run:
|
||||
`ssh-keyscan -t ed25519 {host} | awk '{print $2" "$3}'` and store
|
||||
the output via `oikos secret set ssh/host-keys/{slug} {output}`.
|
||||
- **Risk class**: config_mutation (initial pin) / destructive (if keys change)
|
||||
|
||||
### B6. Fix env var naming inconsistency
|
||||
|
||||
- **Status**: Done (commit pending)
|
||||
- **Current**: Config uses `OIKOS_INFISICAL_*` prefix in docker-compose but
|
||||
`internal/secrets/infisical.go` lines 58–62 falls back to bare `INFISICAL_*`
|
||||
(without OIKOS prefix). Two naming conventions for the same bootstrap vars.
|
||||
- **Fix**: Standardize on `OIKOS_INFISICAL_*` everywhere. Remove the bare
|
||||
`INFISICAL_*` fallback in infisical.go.
|
||||
- **What changed**: Removed the `os.Getenv("INFISICAL_CLIENT_ID")` and
|
||||
`os.Getenv("INFISICAL_CLIENT_SECRET")` fallbacks in `infisical.go connect()`.
|
||||
Removed unused `os` import. Error message updated to reference
|
||||
`OIKOS_INFISICAL_*` names.
|
||||
- **Risk class**: config_mutation
|
||||
|
||||
### B7. Align `secretsBackend` interface with `secrets.Backend`
|
||||
|
||||
- **Status**: Done (commit pending)
|
||||
- **Current**: `internal/httpapi/server.go` lines 62–70 defines a local
|
||||
`secretsBackend` interface (Get/Set/List) that omits `Name()` from the
|
||||
canonical `secrets.Backend`.
|
||||
- **Fix**: Use `secrets.Backend` directly in httpapi, or embed it in the local
|
||||
interface.
|
||||
- **What changed**:
|
||||
- Removed `secretsBackend` interface from `httpapi/server.go`; `Server.secretsManager`
|
||||
now uses `secrets.Backend` directly.
|
||||
- Removed `secretBackend` interface from `mcp/server.go`; `NewHandler` and
|
||||
`newServer` now accept `secrets.Backend`.
|
||||
- Updated `mcp/tools.go` `allTools()` signature to accept `secrets.Backend`.
|
||||
- Updated `mcp/secrets_tools_test.go` mock to implement `secrets.Backend`
|
||||
(added `Name()` method, uses `secrets.ErrNotFound` instead of custom error).
|
||||
- **Risk class**: read_only
|
||||
|
||||
## C. Security fixes (critical, non-Infisical)
|
||||
|
||||
### C1. nomos.hubris.network has no authentication
|
||||
- **Where**: Caddy reverse proxy config — nomos endpoint bypasses forward_auth
|
||||
- **Risk**: Anyone on the mesh/LAN can talk to the AI agent directly, bypassing
|
||||
all policy classification and approval gates.
|
||||
- **Fix**: Add `forward_auth` to the nomos Caddy route, or require the MCP bearer
|
||||
token. At minimum, add a shared secret via Caddy `basicauth`.
|
||||
- **Risk class**: config_mutation
|
||||
|
||||
### C2. pg_dump failure is silently ignored
|
||||
- **Where**: `scripts/deploy.sh` — `pg_dump ... || echo "WARNING"`
|
||||
- **Risk**: Broken backup goes unnoticed until a rollback is needed and fails.
|
||||
- **Fix**: Fail the deploy on pg_dump error, or at minimum send a Matrix alert
|
||||
and refuse to proceed if the dump is empty/corrupt.
|
||||
- **Risk class**: config_mutation
|
||||
|
||||
### C3. CORS defaults to `*`
|
||||
- **Where**: `internal/httpapi/server.go` — `AllowedOrigins: []string{"*"}` when
|
||||
`OIKOS_CORS_ORIGIN` is not set
|
||||
- **Fix**: Default to empty (deny all) or require explicit configuration.
|
||||
- **Risk class**: config_mutation
|
||||
|
||||
## D. Operational improvements (high)
|
||||
|
||||
### D1. Add CI pipeline
|
||||
- **Status**: Done (hardened after review)
|
||||
- **Current**: No automated build/test on push. `make lint test generate-check`
|
||||
exists but is manual.
|
||||
- **Fix**: Add Gitea Actions (or drone) pipeline: `make lint test generate-check`
|
||||
on every push to `main`. Block deploy if pipeline fails.
|
||||
- **What changed**: The Gitea Actions pipeline already exists
|
||||
(`.gitea/workflows/ci.yml`). Added the missing deploy gate as step [1/8] in
|
||||
`scripts/deploy.sh`, run **before** any working-tree mutation: resolves the
|
||||
target SHA read-only via `git ls-remote`, then polls Gitea's combined
|
||||
commit-status API, refusing on `failure`/`error` or a genuine pending-timeout.
|
||||
Hardened across two review passes:
|
||||
- **Deploy lock**: a portable `mkdir`-based lock (macOS has no `flock`) with
|
||||
stale-PID recovery and an `EXIT` trap serializes the webhook's background
|
||||
deploys so a second push during the CI wait fails fast instead of racing.
|
||||
- **TOCTOU guard**: after `git pull --ff-only`, asserts `HEAD ==` the verified
|
||||
SHA (full-SHA compare); aborts if origin/main advanced mid-deploy.
|
||||
- **Token hygiene**: `GITEA_TOKEN` is passed via `curl --config -` (stdin),
|
||||
never in argv/`ps`.
|
||||
- **Misconfig tolerance**: `404`/`401`/`403` or a sustained no-signal streak
|
||||
warn + proceed rather than bricking every deploy; an unset
|
||||
`GITEA_URL`/`GITEA_TOKEN` skips the gate entirely.
|
||||
- **Risk class**: config_mutation
|
||||
|
||||
### D2. Version Docker images
|
||||
- **Status**: Done
|
||||
- **Current**: All images built as `:latest`. Rollback requires full rebuild.
|
||||
- **Fix**: Tag images with `v$VERSION` from the VERSION file in deploy.sh. Keep
|
||||
last 3 versions. Enable `docker compose up` to pin a version tag.
|
||||
- **What changed**: Every built compose service now carries an `image:
|
||||
oikos-<svc>:${OIKOS_VERSION:-latest}` tag. `deploy.sh` exports
|
||||
`OIKOS_VERSION=v$(cat VERSION)` **after** `git pull` (so the tag always
|
||||
matches the built code) and step [6/8] prunes each service to the 3 newest
|
||||
version tags. The prune repo list is derived at runtime from
|
||||
`docker compose config --images` (hardcoded list kept only as a fallback).
|
||||
- **Risk class**: config_mutation
|
||||
|
||||
### D3. Add rate limiting
|
||||
- **Status**: Done
|
||||
- **Current**: No throttling on HTTP API or MCP endpoints. An agent in a loop
|
||||
could hammer the API or exhaust DB connections.
|
||||
- **Fix**: Add `golang.org/x/time/rate` middleware to chi router. Per-IP or
|
||||
per-token rate limit with burst allowance. Separate limits for API vs MCP.
|
||||
- **What changed**: New `internal/httpapi/ratelimit.go` — a per-client (IP)
|
||||
token-bucket registry with a ctx-driven idle-entry sweep (stops its ticker on
|
||||
shutdown). Wired into `NewHandler` before CORS/auth; `/healthz` is exempt.
|
||||
Configurable via `OIKOS_API_RATE_LIMIT`/`OIKOS_API_RATE_BURST`; **unset =
|
||||
disabled** (the default). Returns RFC 9457 429 + Retry-After. `x/time`
|
||||
promoted to a direct dependency. `clientIP` takes the **rightmost** XFF hop
|
||||
(Caddy's appended value); a documented residual limitation is that a direct
|
||||
(non-proxy) connection can still spoof XFF — full closure needs Caddy
|
||||
`trusted_proxies` or per-token keying.
|
||||
- **Risk class**: config_mutation
|
||||
|
||||
### D4. Add container resource limits
|
||||
- **Status**: Done
|
||||
- **Current**: No `mem_limit`, `cpus`, or `ulimits` on any compose service.
|
||||
- **Fix**: Add memory and CPU limits to all services in docker-compose.yml.
|
||||
Suggested: API 512MB, scheduler 256MB, notifier 128MB, nomos 512MB.
|
||||
- **What changed**: Added `mem_limit`/`cpus` to all 10 services: postgres 1g/2,
|
||||
api 512m/1, scheduler 256m/1, notifier 128m/0.5, nomos 512m/1, web 64m/0.25,
|
||||
redis 128m/0.5, infisical 512m/1, migrate/seed 512m/1.
|
||||
- **Risk class**: config_mutation
|
||||
|
||||
### D5. Add healthchecks to all compose services
|
||||
- **Status**: Done
|
||||
- **Current**: Only postgres, api, and redis have healthchecks.
|
||||
- **Fix**: Add `healthcheck` to scheduler, notifier, and nomos. Scheduler can
|
||||
expose a `/healthz` with last-check-timestamp; notifier with last-notify-timestamp.
|
||||
- **What changed**: New `internal/health` package — a staleness-aware probe
|
||||
(`Bump()` per loop iteration; `/healthz` returns 200 within the window, 503
|
||||
once stale). Wired into `scheduler.Run` (:8093, 3× interval) and
|
||||
`notifier.Run` (:8094, 2 min); nomos already served `:8092/healthz`. Added
|
||||
compose healthchecks for scheduler, notifier, and nomos. All long-lived
|
||||
services now have a healthcheck; the probe ports are bound to localhost only.
|
||||
- **Risk class**: config_mutation
|
||||
|
||||
## E. Code quality (medium)
|
||||
|
||||
### E1. Split monolithic files
|
||||
### E1. Split monolithic files
|
||||
- **Status**: Done
|
||||
- **What changed**: All three monoliths split:
|
||||
- `internal/mcp/tools.go` (1774→0 lines): `entity_tools.go`, `ops_tools.go`,
|
||||
`knowledge_tools.go`, `analysis_tools.go` — tools grouped by domain, each
|
||||
with its own handler closures. `tools.go` is now a thin registry.
|
||||
- `internal/httpapi/impl.go` (1533→0 lines): `entities.go`, `events.go`,
|
||||
`signals.go`, `ontology.go`, `fleet_health.go`, `client_context.go`,
|
||||
`client_lifecycle.go`, `entity_mutations.go`, `query_audit.go`.
|
||||
- `cmd/nomos/main.go` (1127→0 lines → renamed to `server.go`): `mcp.go`,
|
||||
`workers.go` split from the monolithic serve function.
|
||||
- **Risk class**: reversible_low (code moves, no behavior change)
|
||||
|
||||
### E2. Migrate raw pool.Exec queries to sqlc
|
||||
- **Status**: Done
|
||||
- **What changed**: Added `/ internal/db/queries/entities.sql` and
|
||||
`relationships.sql` source files with `-- name:` annotations. Generated
|
||||
typesafe Go bindings in `sqlcgen/` (compiled with `go generate`). Migration
|
||||
covers the most-frequently hit entity/relationship queries; remaining raw
|
||||
queries in HTTP/MCP handlers tracked separately.
|
||||
- **Risk class**: reversible_low (query output is identical)
|
||||
|
||||
### E3. Unify SSH implementations
|
||||
### E3. Unify SSH implementations
|
||||
- **Status**: Done (hardened after review)
|
||||
- Scheduler used `os/exec ssh` (system binary), MCP/actuator used `crypto/ssh`.
|
||||
- Unified on `crypto/ssh` with a shared `internal/actuator` package:
|
||||
- `client.go` — `HostKeyCallback`, `LoadSigner` (with per-path signer cache),
|
||||
`Dial`, `RunCombinedOutput`, `RunOutput` (stdout-only, stderr folded into error)
|
||||
- `stream.go` — `streamWriter` + `RunStreaming` (session, goroutine+panic recovery,
|
||||
done/timeout/ctx select, partial output on timeout)
|
||||
- Both `mcp/server.go` and `httpapi/actuator.go` delegate to `actuator.RunStreaming`;
|
||||
the scheduler's `sshExec` uses `actuator.Dial` + `actuator.RunOutput`.
|
||||
- **Review fixes applied**:
|
||||
- `RunOutput` preserves pre-E3 `exec.Cmd.Output()` semantics (scheduler parses
|
||||
stdout as JSON/string, not interleaved combined output)
|
||||
- `sshKeyPath` deploy fallback (`$SSH_KEY_PATH` → `$HOME/.ssh/id_rsa`) restored
|
||||
- Duplicate `sshExecStream`/`streamWriter` (83-line verbatim copies in mcp + httpapi)
|
||||
consolidated into `actuator/stream.go`
|
||||
- `LoadSigner` caches parsed keys per keyPath (avoids re-reading 100+/cycle)
|
||||
- `RunOutput` includes captured stderr in the error message on failure
|
||||
|
||||
### E4. Fix lifecycle attribute check
|
||||
- **Status**: Done
|
||||
- `internal/db/lifecycle.go`: `checkPrecondition` used `strings.Contains(attrs, want)`
|
||||
on raw JSONB text, bypassing the GIN index.
|
||||
- **Fix**: Extracted `fetchAttrs` + `attrTruthy` helpers that parse JSONB with `json.Unmarshal`
|
||||
and use `@>` JSONB operator for precondition queries. Added `lifecycle_test.go` with
|
||||
9+2 table-driven cases.
|
||||
|
||||
### E5. Add table-driven tests for core logic
|
||||
- **Status**: Done
|
||||
Packages covered (previously 0%):
|
||||
1. `internal/policy` — risk_test.go (62.9% → 64.7%)
|
||||
2. `internal/ontology` — preconditions_test.go (50.5% → 63.1%)
|
||||
3. `internal/checkdefaults` — build_test.go (26.5% → 52.5%)
|
||||
4. `internal/actuator` — client_test.go (SSH key parsing, RunOutput)
|
||||
5. `internal/db` — lifecycle_test.go (attrTruthy, precondition SQL)
|
||||
|
||||
## F. Performance (medium)
|
||||
|
||||
### F1. SSH connection pooling for scheduler
|
||||
- Migrated from `actuator.Dial()` (new TCP+SSH per check) to `actuator.DialPool`
|
||||
with key-by-host pooling and 5min idle TTL. One TCP connection per Proxmox host
|
||||
multiplexes sessions for all concurrent checks targeting that host (F1).
|
||||
- **New files**: `internal/actuator/pool.go` — thread-safe pool with lazy dial,
|
||||
duplicate-suppression on race, and periodic idle eviction.
|
||||
- **Changed**: `internal/scheduler/scheduler.go` — `Run()` initializes the pool
|
||||
(deferred `Close()`), `sshExec` calls `pool.Get()` instead of `Dial()`, and
|
||||
no longer calls `client.Close()` (the pool owns the lifecycle).
|
||||
|
||||
### F2. Entity lookup cache
|
||||
- Added `internal/db/entity_cache.go` — a `sync.RWMutex`-guarded TTL map keyed
|
||||
by both slug and ID string with 60s expiry. HTTP API `resolveEntityID` checks
|
||||
the cache before hitting the DB; `PatchEntity` invalidates on write.
|
||||
- The MCP path (`queryEntity`) is not cached since MCP calls are already
|
||||
rate-limited and less frequent than the HTTP API.
|
||||
|
||||
### F3. Trigram index for entity search
|
||||
- **Migration**: `migrations/031_entity_trigram_index.up.sql` — creates `pg_trgm`
|
||||
extension and GIN trigram indexes on `entities.slug` and `entities.name` so
|
||||
that `ILIKE '%'||q||'%'` scans use index lookups instead of sequential scans.
|
||||
|
||||
### F4. Composite index for auto-act anti-join
|
||||
- **Migration**: `migrations/032_auto_act_index.up.sql` — creates a partial index
|
||||
`idx_executions_classification` on `executions(classification_id)` where
|
||||
`entity_id IS NOT NULL`, supporting the `LEFT JOIN ... WHERE e.entity_id IS NULL`
|
||||
anti-join in `GetOpenSignalsForAutoAct`.
|
||||
|
||||
## G. Observability (low)
|
||||
|
||||
### G1. Add OpenTelemetry tracing
|
||||
- OTel SDK is already in go.mod as indirect dependency.
|
||||
- Instrument HTTP handlers, MCP tools, and DB queries with spans.
|
||||
- Propagate trace context via `correlation_id` (already exists in audit/events).
|
||||
|
||||
### G2. Prometheus metrics export
|
||||
- Expose `/metrics` endpoint for Go runtime, DB pool stats, scheduler check
|
||||
duration/counts, HTTP request latency histograms.
|
||||
- Complement the existing TimescaleDB metric_samples (which are entity health
|
||||
metrics, not self-observability).
|
||||
|
||||
### G3. Automate offsite backups
|
||||
- Proton Drive backup target entity exists but no pipeline.
|
||||
- Add `rclone cron` to `pg_dump | zstd | rclone sync` to Proton Drive.
|
||||
- Weekly backup verification (restore to test DB, run `make test-db`).
|
||||
|
||||
## H. Infrastructure (low)
|
||||
|
||||
### H1. Pin Infisical image version
|
||||
- **Done** — `docker-compose.yml` pinned `infisical/infisical:latest` → `v0.99.1`.
|
||||
Unlike other compose services (which use `${OIKOS_VERSION}` from the repo),
|
||||
Infisical is a prebuilt upstream image and needs a hardcoded tag.
|
||||
|
||||
### H2. Add persistent job queue for executions
|
||||
- **Done** — New `internal/execworker/` package implements a Postgres-backed
|
||||
queue daemon. Polls every 15s for executions with `status IN ('proposed',
|
||||
'pending_approval')`, acquires a per-execution `pg_try_advisory_lock` for
|
||||
at-most-once delivery, resolves the SSH target via `remote.ResolveHost`,
|
||||
and runs the action command via `actuator.RunCombinedOutput`.
|
||||
- On startup, recovers orphaned `status='running'` executions (crashed workers)
|
||||
by marking them as `failed`.
|
||||
- Registered as an `execution-worker` role in `cmd/oikos/main.go` and wired
|
||||
into both the standalone (`oikos execution-worker`) and `case "all"` runner.
|
||||
- Added to `docker-compose.yml` as a service with SSH key volume mount,
|
||||
liveness probe, and `profiles: ["dev", "full"]`.
|
||||
- **Files**: `internal/execworker/worker.go`, `internal/execworker/init.go`,
|
||||
`cmd/oikos/main.go` (new role + "all" background), `docker-compose.yml` (service).
|
||||
|
||||
### H3. Replace or harden custom migration splitter
|
||||
- **Done** — `splitSQL()` in `internal/db/pool.go` now handles block
|
||||
comments (`/* */`) and single-quoted string literals (`'...'`) in
|
||||
addition to the existing dollar-quote and line-comment support.
|
||||
Added 6 new test cases covering: semicolons inside string literals,
|
||||
`$` inside strings, block comments, block comments with dollar signs,
|
||||
doubled SQL quotes (`''`), and empty/no-semicolon inputs.
|
||||
Total: 11 tests, all passing.
|
||||
|
||||
### H4. Add distributed locking for scheduler
|
||||
- **Done** — `scheduler.Run()` acquires `pg_advisory_lock(0x01c05e6)` at
|
||||
startup on a dedicated held connection; if the lock is held by another
|
||||
instance it logs and exits. Released on shutdown via defer (using
|
||||
`context.WithoutCancel` so the unlock runs even when ctx is cancelled).
|
||||
Lock key `0x01c05e6` differs from the migration lock `0x01c05e5`.
|
||||
|
||||
---
|
||||
|
||||
## Execution order
|
||||
|
||||
1. **Phase 0 — Infisical consolidation** (B1–B7): Wire Infisical into all
|
||||
binaries, migrate secrets from env/plaintext, activate SOPS fallback, remove
|
||||
`.env` secrets and plist HMAC. This is the foundation — every subsequent
|
||||
secret-dependent change (SSH host keys in B5, rate limit config, etc.) goes
|
||||
through Infisical. **Do this first.**
|
||||
2. **Phase 1 — Security** (C1–C3, B5): Nomos auth, pg_dump failure, CORS default,
|
||||
SSH host keys (now stored in Infisical per B5).
|
||||
3. **Phase 2 — Operational** (D1–D5): CI pipeline, image versioning, rate
|
||||
limiting, resource limits, healthchecks. **Done.**
|
||||
4. **Phase 3 — Code quality** (E1–E5): File splits, sqlc migration, SSH
|
||||
unification, lifecycle fix, tests. **Done.** (Rebased onto main 0.28.5 and
|
||||
landed as 0.29.0.)
|
||||
5. **Phase 4 — Performance** (F1–F4): SSH pooling, entity cache, trigram
|
||||
index, auto-act index. **Done.**
|
||||
6. **Phase 5 — Observability** (G1–G3): OTel tracing, Prometheus, offsite backups.
|
||||
7. **Phase 6 — Infrastructure** (H1–H4): Pin images, job queue, migration runner,
|
||||
distributed locking. **Done.**
|
||||
|
||||
Phases 0–6 are complete. Phase 5 (Observability) is backlog.
|
||||
|
||||
---
|
||||
|
||||
## Phase 0 post-deploy checklist
|
||||
|
||||
Run these on mac-mini after deploying the Phase 0 code changes.
|
||||
|
||||
### Step 1: Populate Infisical with secrets
|
||||
|
||||
For each secret, read the current value from `.env` and store it in Infisical:
|
||||
|
||||
```bash
|
||||
# Values from .env — read them first, then set
|
||||
oikos secret set matrix/token "$(grep OIKOS_MATRIX_TOKEN .env | cut -d= -f2-)"
|
||||
oikos secret set approval/hmac-secret "$(grep OIKOS_APPROVAL_HMAC_SECRET .env | cut -d= -f2-)"
|
||||
oikos secret set mcp/bearer-token "$(grep OIKOS_MCP_BEARER_TOKEN .env | cut -d= -f2-)"
|
||||
oikos secret set api/token "$(grep OIKOS_API_TOKEN .env | cut -d= -f2-)"
|
||||
oikos secret set oidc/client-secret "$(grep OIKOS_OIDC_CLIENT_SECRET .env | cut -d= -f2-)"
|
||||
oikos secret set openrouter/api-key "$(grep OPENROUTER_API_KEY .env | cut -d= -f-)"
|
||||
oikos secret set webhook/hmac-secret "$(grep WEBHOOK_HMAC_SECRET .env | cut -d= -f2-)"
|
||||
```
|
||||
|
||||
Verify: `oikos secret list` should show all 8 keys.
|
||||
|
||||
### Step 2: Pre-populate SSH host keys (optional, skip if TOFU is acceptable)
|
||||
|
||||
```bash
|
||||
# For each Proxmox host, scan and store the public key
|
||||
for host in pve1 pve2; do
|
||||
key=$(ssh-keyscan -t ed25519 $host 2>/dev/null | awk '{print $2" "$3}')
|
||||
oikos secret set "ssh/host-keys/$host" "ssh-ed25519 $key"
|
||||
done
|
||||
```
|
||||
|
||||
Alternatively, skip this step — the first deployment will TOFU-accept all current
|
||||
host keys and persist them to Infisical automatically.
|
||||
|
||||
### Step 3: Remove HMAC secret from webhook plist
|
||||
|
||||
On mac-mini:
|
||||
```bash
|
||||
sudo launchctl unload ~/Library/LaunchAgents/network.hubris.oikos-deploy-webhook.plist
|
||||
# Edit the plist: remove the <key>WEBHOOK_HMAC_SECRET</key> block
|
||||
sudo launchctl load ~/Library/LaunchAgents/network.hubris.oikos-deploy-webhook.plist
|
||||
```
|
||||
|
||||
### Step 4: Strip secrets from .env
|
||||
|
||||
Edit `.env` to remove the 7 migrated secrets, keeping only bootstrap and
|
||||
non-secret config:
|
||||
|
||||
```bash
|
||||
# Remove these lines:
|
||||
# INFISICAL_ENCRYPTION_KEY=...
|
||||
# OIKOS_MATRIX_TOKEN=...
|
||||
# OIKOS_INFISICAL_CLIENT_ID=...
|
||||
# OIKOS_INFISICAL_CLIENT_SECRET=...
|
||||
# OIKOS_INFISICAL_PROJECT_ID=...
|
||||
# OPENROUTER_API_KEY=...
|
||||
# OIKOS_MCP_BEARER_TOKEN=...
|
||||
|
||||
# Keep these (bootstrap / non-secret):
|
||||
# OIKOS_DATABASE_URL=...
|
||||
# OIKOS_API_LISTEN=...
|
||||
# OIKOS_INFISICAL_SITE_URL=...
|
||||
# OIKOS_INFISICAL_ENV=...
|
||||
```
|
||||
|
||||
### Step 5: Verify
|
||||
|
||||
1. `oikos secret list` — 8 keys + SSH host keys
|
||||
2. `docker compose logs api | grep "secrets resolved"` — should show count=5
|
||||
3. Trigger a test deploy — webhook should still validate HMAC signatures
|
||||
4. `nomos` should start and resolve secrets from Infisical (check logs for
|
||||
`nomos: secrets resolved from Infisical`)
|
||||
5. Verify no secrets appear in process env: `docker compose exec api env |
|
||||
should not show `OIKOS_MCP_BEARER_TOKEN`, `OIKOS_MATRIX_TOKEN`,
|
||||
etc. (they come from Infisical at startup, not env)
|
||||
702
plans/done/2026-08-15-hexagonal-architecture.md
Normal file
702
plans/done/2026-08-15-hexagonal-architecture.md
Normal file
@@ -0,0 +1,702 @@
|
||||
# Hexagonal architecture for Oikos — design and phased refactor plan
|
||||
|
||||
**Date:** 2026-08-15
|
||||
**Status:** Complete — all 10 phases shipped (2026-08-16)
|
||||
**Scope:** All Go code (`cmd/oikos`, `cmd/nomos`, `cmd/webhook`) and the UI
|
||||
split. One hexagon covers the oikos backend; nomos is an external agent
|
||||
client that gets an internal cleanup (Phase 8) but stays outside the core.
|
||||
Phase 1 extracts the client — web SPA, desktop wrapper, web image — into its
|
||||
own repository, making oikos backend-only before the hexagon refactor churns
|
||||
the tree.
|
||||
|
||||
---
|
||||
|
||||
## 1. Summary
|
||||
|
||||
Adopt a hexagonal (ports & adapters) architecture across the Oikos backend.
|
||||
The domain core (entities, signals, executions, policy, knowledge) becomes a
|
||||
pure package with no infrastructure imports. All I/O moves behind named ports
|
||||
implemented by adapters (Postgres, SSH, MCP, HTTP, probes, Infisical/SOPS).
|
||||
REST and MCP — currently two parallel silos each embedding raw SQL —
|
||||
become thin driving adapters over one shared application-service layer.
|
||||
|
||||
No API, MCP-tool, DB-schema, or wire-format changes. The client (web SPA +
|
||||
desktop wrapper) moves to its own repository in Phase 1; the backend
|
||||
restructuring is delivered in nine further shippable phases (0, 2–9).
|
||||
|
||||
---
|
||||
|
||||
## 2. Current state (grounding)
|
||||
|
||||
What the code looks like today, with references:
|
||||
|
||||
| Finding | Evidence |
|
||||
|---|---|
|
||||
| Pure domain package already exists | `internal/domain` — `Entity`, `Signal`, `Execution`, `Approval`, sentinel errors; imports only stdlib |
|
||||
| One working port/adapter pair | `internal/secrets/backend.go:15` `Backend` interface; `InfisicalBackend`, `SOPSBackend`, caching `Manager` |
|
||||
| Business logic lives in HTTP handlers | `internal/httpapi/entities.go` embeds raw SQL in `ListEntities`/`GetGraph`; `Server` struct (`server.go:54`) holds `*db.Pool`, cache, SSE broker |
|
||||
| Business logic lives in MCP handlers | `internal/mcp/entity_tools.go:16` — every `*Tools(pool *db.Pool, ...)` builds tools over direct SQL; `server.go:754` `classifyAndGate` (~300 lines) mixes classification, approval creation, audit, SSH dispatch |
|
||||
| Scheduler is a monolith | `internal/scheduler/scheduler.go` (~1100 LOC) — probe switch at line 331 (`http`, `tcp`, `disk`, `cert-expiry`, `vm-status`, `ping`, `ssh-script`, `backup-freshness`, `dns`), health aggregation, signal upserts, metric writes inline |
|
||||
| Inverted dependency | `internal/db` imports `checkdefaults` (seed ingest knows about check derivation); `actuator` imports `db` |
|
||||
| Duplicated port | `mcp/server.go` defines a local `secretBackend` subset of `secrets.Backend` |
|
||||
| No composition root | wiring is scattered: `httpapi.NewHandler` builds secrets + SSE; `cmd/oikos/main.go` builds the rest |
|
||||
| nomos is a flat `main` package | `cmd/nomos/*.go` — agent, store (direct pgx), MCP client, HTTP server, turn gating all in one package |
|
||||
|
||||
Layered as-is (arrows = imports):
|
||||
|
||||
```mermaid
|
||||
flowchart TD
|
||||
subgraph delivery["Delivery — contains the use-cases today"]
|
||||
httpapi["httpapi (~3500 LOC, 30 files)"]
|
||||
mcp["mcp (~2100 LOC, 67 tools, classifyAndGate)"]
|
||||
end
|
||||
subgraph execution["Execution"]
|
||||
scheduler["scheduler (~1100 LOC, one file)"]
|
||||
execworker["execworker"]
|
||||
actuator["actuator (SSH + breaker)"]
|
||||
end
|
||||
subgraph domainsvc["Domain services"]
|
||||
checkdefaults["checkdefaults"]
|
||||
learning["learning"]
|
||||
knowledge["knowledge"]
|
||||
execlog["execlog"]
|
||||
observability["observability"]
|
||||
end
|
||||
subgraph dbinfra["DB infra"]
|
||||
db["db + sqlcgen"]
|
||||
remote["remote"]
|
||||
audit["audit"]
|
||||
end
|
||||
subgraph nearpure["Near-pure"]
|
||||
ontology["ontology"]
|
||||
policy["policy"]
|
||||
secrets["secrets"]
|
||||
end
|
||||
subgraph pure["Pure (stdlib only)"]
|
||||
domain["domain"]
|
||||
config["config"]
|
||||
safego["safego"]
|
||||
health["health"]
|
||||
end
|
||||
|
||||
httpapi --> db
|
||||
httpapi --> actuator
|
||||
httpapi --> mcp
|
||||
httpapi --> secrets
|
||||
mcp --> db
|
||||
mcp --> actuator
|
||||
mcp --> execlog
|
||||
mcp --> policy
|
||||
mcp --> remote
|
||||
mcp --> checkdefaults
|
||||
mcp --> audit
|
||||
scheduler --> db
|
||||
scheduler --> actuator
|
||||
scheduler --> remote
|
||||
execworker --> db
|
||||
execworker --> actuator
|
||||
execworker --> remote
|
||||
actuator --> secrets
|
||||
checkdefaults --> ontology
|
||||
ontology --> domain
|
||||
policy --> domain
|
||||
db --> checkdefaults
|
||||
remote --> db
|
||||
audit --> db
|
||||
learning --> db
|
||||
knowledge --> db
|
||||
execlog --> observability
|
||||
observability --> db
|
||||
```
|
||||
|
||||
The problem: use-cases have no home. Every new consumer (REST tool, MCP tool,
|
||||
scheduler) re-implements or copy-pastes query + policy + audit logic.
|
||||
|
||||
---
|
||||
|
||||
## 3. Target architecture
|
||||
|
||||
### 3.1 System context
|
||||
|
||||
One hexagon. nomos, the Gitea `webhook` receiver, and the desktop shell stay
|
||||
outside — nomos and the desktop shell are external clients of the core (over
|
||||
MCP/REST), the webhook is a leaf deploy utility:
|
||||
|
||||
```mermaid
|
||||
flowchart LR
|
||||
subgraph ext["External clients"]
|
||||
spa["web SPA"]
|
||||
desktop["desktop shell"]
|
||||
curl["curl / scripts"]
|
||||
nomos["nomos agent"]
|
||||
timers["timers"]
|
||||
end
|
||||
|
||||
subgraph oikos["OIKOS CORE HEXAGON"]
|
||||
subgraph driving["Driving adapters (left side of the hexagon)"]
|
||||
httpapi["httpapi — REST"]
|
||||
mcpserver["mcpserver — MCP tools"]
|
||||
schedad["scheduler — timer"]
|
||||
execworker["execworker — queue poller"]
|
||||
cli["cli — seed/export/secret"]
|
||||
end
|
||||
subgraph core["Core"]
|
||||
app["app services (use-cases)"]
|
||||
dom["core/domain (pure model)"]
|
||||
end
|
||||
subgraph driven["Driven adapters (right side of the hexagon)"]
|
||||
postgres["postgres — repositories"]
|
||||
ssh["ssh — CommandExecutor"]
|
||||
probes["probes/* — Checker per kind"]
|
||||
remote["remote — TargetResolver"]
|
||||
secretsad["secrets — Infisical/SOPS"]
|
||||
events["events — EventPublisher/SSE"]
|
||||
end
|
||||
end
|
||||
|
||||
subgraph leaves["Leaf utilities — unchanged"]
|
||||
webhook["webhook — Gitea push → deploy"]
|
||||
end
|
||||
|
||||
spa -- REST --> httpapi
|
||||
desktop -- REST --> httpapi
|
||||
curl -- REST --> httpapi
|
||||
nomos -- MCP --> mcpserver
|
||||
timers -- tick --> schedad
|
||||
|
||||
driving --> app
|
||||
app --> dom
|
||||
app -- "via ports" --> driven
|
||||
|
||||
webhook -. "docker compose on mac-mini" .-> oikos
|
||||
```
|
||||
|
||||
Dependency rules (enforced with `depguard` from Phase 0):
|
||||
|
||||
1. **Core imports nothing from adapters; adapters and `cmd/*` import core.**
|
||||
Adapter→adapter imports are allowed only via ports (e.g. the
|
||||
target-resolver adapter consumes `EntityRepository`, not the postgres
|
||||
package).
|
||||
2. **The core never links agent-client tech:** `internal/core` must not import
|
||||
`internal/nomos`, the OpenAI SDK, or MCP *client* packages. nomos consumes
|
||||
oikos exclusively through its public MCP/REST surface — over the wire, not
|
||||
through shared packages.
|
||||
3. **nomos is external:** `internal/nomos` must not import `internal/core` or
|
||||
`internal/adapters`. Its tables (`agent_sessions`, `agent_messages`,
|
||||
`session_plan_steps`, `nomos_plan_executions`) stay in the shared
|
||||
migrations tree — owned operationally by the oikos deploy, accessed by
|
||||
nomos only through its own store code.
|
||||
|
||||
### 3.2 Target directory tree
|
||||
|
||||
```
|
||||
internal/
|
||||
core/
|
||||
domain/ moved from internal/domain (pure, unchanged)
|
||||
app/ application services (use-cases) — see §3.4
|
||||
ports/ driven-port interfaces — see §3.3
|
||||
adapters/
|
||||
postgres/ pool, migrations, sqlcgen, repository impls
|
||||
ssh/ from internal/actuator: dial pool, breaker, streaming,
|
||||
provisioning (pct/qm)
|
||||
remote/ target resolver impl (from internal/remote), built on
|
||||
ports.EntityRepository — no direct postgres import
|
||||
probes/ one file per check kind (http, tcp, dns, cert, ping,
|
||||
sshscript, vmstatus, backup)
|
||||
secrets/ from internal/secrets (Backend interface moves to
|
||||
core/ports; Infisical/SOPS/Manager stay here)
|
||||
events/ EventPublisher adapter: SSE broker + events table
|
||||
httpapi/ REST driving adapter (handlers, auth, SSE endpoint)
|
||||
mcpserver/ from internal/mcp — tool schema + arg mapping only
|
||||
scheduler/ thin timer adapter — ticks ObservationService
|
||||
execworker/ thin poller adapter — claims queued executions
|
||||
cli/ oikos seed/export/secret subcommands
|
||||
config/ unchanged (pure stdlib — legitimately importable by
|
||||
adapters, not only by cmd/* composition roots)
|
||||
internal/nomos/ Phase 8: nomos internals — plain packages, no hexagon
|
||||
session/ chat sessions, plan execution logic
|
||||
turngate/ retrycap/ messagequeue/ assent/
|
||||
ports.go local ports: LLMClient, HomelabClient (MCP),
|
||||
SessionStore — defined here, not in core/ports
|
||||
cmd/
|
||||
oikos/ composition root: build adapters → services → adapters
|
||||
nomos/ composition root: wires openai-go, MCP client, pgx store
|
||||
into internal/nomos ports
|
||||
webhook/ unchanged leaf
|
||||
desktop/ unchanged leaf (UI client)
|
||||
```
|
||||
|
||||
### 3.3 Ports catalog (driven — core declares, adapters implement)
|
||||
|
||||
Command-side repository methods are **transaction-scoped aggregates**: one
|
||||
method = one `BEGIN…COMMIT` = everything that must succeed or fail together
|
||||
(see §3.6). Read methods are plain queries.
|
||||
|
||||
| Port | Key methods | Consumed by | Adapter(s) | Origin in current code |
|
||||
|---|---|---|---|---|
|
||||
| `EntityRepository` | Get/bySlug, List(filters), Search (reads); `Create(CreateInput)`, `Update(UpdateInput)`, `SetState(TransitionInput)` — each input carries derived checks + audit + event, committed atomically | EntityService, GraphService, resolver, seeds | postgres | raw SQL in `httpapi/entities.go`, `mcp/entity_tools.go`; `db/sqlcgen` |
|
||||
| `RelationshipRepository` | Create, End, ListFor(entity, dir); endpoint validation via OntologyStore happens in core before Create | GraphService | postgres | `httpapi/relationships.go`, `mcp` relation tools |
|
||||
| `OntologyStore` | LoadTypeTree (types, rel types, lifecycles), cached | EntityService, GraphService, PolicyService | postgres + in-memory cache | `ontology.TypeTree` built from `db` |
|
||||
| `CheckRepository` | ListEnabled, ListFor(entity) (reads); `EnsureFor(entity, desiredDefs)` — read-diff-write in one tx; `SetEnabled` | MonitoringService, ObservationService | postgres | `scheduler.go`, `httpapi/checks.go`, `db/checks.go` |
|
||||
| `SignalRepository` | Open, History (reads); `UpsertWithTriggers(upsertInput)` — check-then-act on signal state in one tx; `Transition(ack/resolve/mute)` | SignalService, ObservationService | postgres | `scheduler.go` signal upserts, `httpapi/signals.go` |
|
||||
| `ExecutionRepository` | List(cursor), ReadLog (reads); `SubmitQueued(SubmitInput)` — execution + approval + audit + event in one tx; `Claim(next)` — advisory-lock claim; `AppendLog`; `Complete(CompleteInput)` | ExecutionService | postgres | `execworker`, `httpapi/executions.go`, `execlog`, `mcp/server.go` |
|
||||
| `ApprovalRepository` | ListPending (read); `Decide(DecideInput)` — token verification (check-then-act) + approval status + gated-execution status + audit in one tx | ApprovalService | postgres | `httpapi/approvals.go:94-145`, `mcp/server.go` approval insert |
|
||||
| `MetricsRepository` | `InsertSamples` (write path) | ObservationService | postgres (Timescale) | `scheduler.go`; bucketed/trend queries live in `ReadModels` |
|
||||
| `AuditRepository` | AppendAudit, AppendEvent (command side-effects, usually passed into other inputs) | AuditService, all services | postgres | `observability`, `audit` |
|
||||
| `KnowledgeRepository` | Search, GetContent, Revisions, Tags, Orphans, Duplicates (reads); `Upsert(UpsertInput)` — row + revision + about-edges in one tx; Merge, SoftDelete/Restore | KnowledgeService | postgres | `httpapi/knowledge*.go`, `mcp/knowledge_tools.go` |
|
||||
| `LearningRepository` | ListFeedback, ListPatterns, ListSkills (reads); `UpsertPattern`, `Validate/Quarantine` | LearningService | postgres | `learning`, `httpapi/patterns.go` |
|
||||
| `ReadModels` | Query-shaped reads for report endpoints: graph, fleet health, dashboard, metric buckets, trends, audit trail, event timeline, agent activity, drift reports, learning views | httpapi + mcpserver adapters **directly** — no service hop (see §3.4) | postgres | report SQL currently inline in `httpapi/*` handlers |
|
||||
| `CommandExecutor` | Run(target, cmd, opts) streaming/combined; returns exit code | ExecutionService, probe adapter (ssh-script), ProvisioningService | ssh (dial pool + circuit breaker) | `actuator.RunStreaming/RunCombinedOutput` |
|
||||
| `TargetResolver` | ResolveExecTarget(slug), ResolveForCheck, ResolveHost, IsGuest | ExecutionService, probes, ssh adapter | remote (built on EntityRepository) | `remote/remote.go` |
|
||||
| `Secrets` | Get, List, Set, Name | config overlay, ExecutionService, ssh (signers, host keys) | infisical, sops, manager | `secrets.Backend` (interface moves to ports; delete mcp's local copy) |
|
||||
| `EventPublisher` | Publish(ctx, Event) | all services | events (SSE broker + events table, LISTEN/NOTIFY) | `httpapi/sse.go` broker + `observability.Event` |
|
||||
| `Checker` | Check(ctx, CheckDef, resolved target) → result{value, state, msg} | ObservationService | probes/http, probes/tcp, probes/dns, probes/cert, probes/ping, probes/sshscript, probes/vmstatus, probes/backup, probes/disk | `scheduler.go:331` kind switch |
|
||||
| `Provisioner` | CreateLXC, CreateVM (pct/qm flows) | ProvisioningService | ssh (proxmox commands) | actuator provisioning files |
|
||||
|
||||
Driving adapters (outside → core): `httpapi` (REST, SPA/desktop/curl),
|
||||
`mcpserver` (67 tools, nomos and any MCP agent), `scheduler` (timer),
|
||||
`execworker` (queue poller), `cli` (seed/export/secret), `sse` endpoint
|
||||
(read side). nomos, the Gitea `webhook`, and the desktop shell stay outside
|
||||
the hexagon — external clients and a leaf utility respectively.
|
||||
|
||||
### 3.4 Application services (core/app)
|
||||
|
||||
| Service | Use-cases | Absorbs logic from |
|
||||
|---|---|---|
|
||||
| `EntityService` | create/update/merge entities, lifecycle transitions (ontology-validated), check derivation on attribute change, enrollment | `httpapi/entity_mutations.go`, `mcp` create/update tools, `checkdefaults` |
|
||||
| `GraphService` | relations CRUD, graph read model, blast radius, infra drift discovery, knowledge-graph audit | `httpapi/entities.go` graph SQL, `mcp/discover.go`, `audit` |
|
||||
| `MonitoringService` | check-def CRUD, enable/disable, defaults | `httpapi/checks.go`, `default_checks.go` |
|
||||
| `ObservationService` | one observe pass: load enabled checks, resolve targets, run probes (Checker port) under bounded worker-pool concurrency (10 — `scheduler.go:133` `SetLimit`; the cap is a service contract, not a timer detail), aggregate health, transition signals, record metrics, sweep staleness | `scheduler/scheduler.go` |
|
||||
| `SignalService` | ack/resolve/mute, history, triggers | `httpapi/signals.go` |
|
||||
| `PolicyService` | classify command/signal, preflight, autonomy rules (classifier can only lower autonomy) | `policy`, half of `classifyAndGate` |
|
||||
| `ExecutionService` | submit (classify → gate → auto-run or queue), status, streaming logs, cancel | other half of `classifyAndGate`, `httpapi/executions.go`, `execworker` dispatch |
|
||||
| `ApprovalService` | list pending, decide → resume queued execution | `httpapi/approvals.go` |
|
||||
| `KnowledgeService` | search, upsert with revisions + entity links, tags, merge, drift | `httpapi/knowledge*.go`, `mcp/knowledge_tools.go` |
|
||||
| `LearningService` | pattern extraction (Wilson confidence), feedback, skills | `learning` |
|
||||
| `AuditService` | audit trail, event timeline, agent activity, drift report | `httpapi/audit.go`, `events.go`, `activity.go`, `audit` |
|
||||
| `SecretsService` | get/list; set routes through approval flow | `mcp/secrets` tools |
|
||||
| `ProvisioningService` | LXC/VM create with provisioning_steps tracking | actuator pct/qm paths, `httpapi/pct_create_test.go` flow |
|
||||
| `SeedService` | seed ingest (ontology, inventory, policy, knowledge) + export to YAML | `db/seed.go`, `db/export.go` — fixes the `db → checkdefaults` inversion |
|
||||
|
||||
Services take ports as constructor arguments; they return `domain` types and
|
||||
sentinel errors. Presenters (JSON shapes, MCP tool results) stay in the
|
||||
adapters. REST and MCP keep their existing wire formats — mapping code just
|
||||
moves to the adapters.
|
||||
|
||||
**Reads bypass services.** Roughly half of `httpapi` and several MCP tools
|
||||
are invariant-free reports (metrics, audit trail, event timeline, dashboard,
|
||||
fleet health, learning views). Those adapters call the `ReadModels` port
|
||||
directly — no SQL in handlers, no service hop, no ceremony. Services exist
|
||||
only where invariants, policy, or multi-step coordination apply (the command
|
||||
side). This is the CQRS-lite line: commands flow through `core/app`, reads
|
||||
flow adapter → `ReadModels` → presenter.
|
||||
|
||||
### 3.5 Composition
|
||||
|
||||
`cmd/oikos/main.go` becomes the single composition root per role:
|
||||
|
||||
```
|
||||
pool := postgres.Connect(...)
|
||||
repos := postgres.NewRepositories(pool)
|
||||
resolver := remote.New(repos.Entities)
|
||||
sshExec := ssh.NewExecutor(secretsMgr, resolver)
|
||||
events := events.NewPublisher(pool, broker)
|
||||
execSvc := app.NewExecutionService(repos.Executions, repos.Approvals,
|
||||
policySvc, sshExec, events, repos.Audit)
|
||||
httpH := httpapi.New(cfg, entitySvc, graphSvc, execSvc, ..., readModels)
|
||||
mcpH := mcpserver.New(cfg, entitySvc, graphSvc, execSvc, ..., readModels)
|
||||
```
|
||||
|
||||
`httpapi` stops mounting business deps; it mounts `mcpserver`'s handler at
|
||||
`/mcp` as pure routing (or main mounts both on one chi router — Phase 3
|
||||
detail, recommend main owns the router).
|
||||
|
||||
**Startup order (composition root contract):**
|
||||
|
||||
1. Load env config (`internal/config` — pure, importable by adapters).
|
||||
2. Build secrets manager and overlay config — *before* the DB pool. Note the
|
||||
chicken-egg: `INFISICAL_ENCRYPTION_KEY` bootstraps Infisical itself and
|
||||
cannot live in Infisical; it stays in env/`.env`.
|
||||
3. Connect pool, run repositories, resolver, executor, events.
|
||||
4. Build services, then driving adapters; start background loops last.
|
||||
|
||||
**Shutdown order:** cancel root ctx (stops the events adapter's dedicated
|
||||
LISTEN/NOTIFY connection and SSE broker) → **then** close the pool —
|
||||
reversing this deadlocks `pool.Close()` on the held connection (constraint
|
||||
documented at `httpapi/server.go:66-70`; ownership moves to the events
|
||||
adapter).
|
||||
|
||||
### 3.6 Transaction and consistency strategy
|
||||
|
||||
**Decision (review F1): aggregate-scoped repository methods — no UnitOfWork
|
||||
port, no `pgx.Tx` in core.** One repository method = one transaction =
|
||||
everything that must succeed or fail together.
|
||||
|
||||
Current code protects invariants with multi-statement tx blocks in 20+
|
||||
places (`pool.Begin` sites). The load-bearing ones:
|
||||
|
||||
- approval decide: HMAC token check-then-act + approval status + gated
|
||||
execution status + audit (`httpapi/approvals.go:94-145`) — double-approve
|
||||
must not double-execute a `destructive` command
|
||||
- entity state transition: lifecycle precondition check-then-act + update +
|
||||
audit + event (`httpapi/entity_mutations.go:200-274`)
|
||||
- check derivation read-diff-write (`db/checks.go:21`)
|
||||
- execution claim (advisory lock), signal upsert transitions, seed ingest
|
||||
|
||||
Mechanics:
|
||||
|
||||
1. Core does the pure work first: validate against the cached OntologyStore,
|
||||
derive desired checks from the TypeTree (`checkdefaults` logic, pure),
|
||||
classify risk, build audit/event entries.
|
||||
2. Core passes **one complete input struct** — e.g.
|
||||
`EntityRepository.Create(ctx, CreateInput{Entity, DerivedChecks, Audit, Event})`.
|
||||
3. The postgres adapter runs `BEGIN → writes → COMMIT` internally and returns
|
||||
the result; partial failures roll back exactly as today.
|
||||
4. Cross-aggregate operations that transact together today stay one method
|
||||
(`ApprovalRepository.Decide` spans approvals + executions + audit).
|
||||
|
||||
What is explicitly rejected:
|
||||
|
||||
- UnitOfWork/Tx-manager port: every port doubles into tx/non-tx variants,
|
||||
fakes multiply, `pgx.Tx` leaks into the core this refactor exists to protect.
|
||||
- Fine-grained autocommit ports: silently deletes the check-then-act
|
||||
guarantees (token double-spend, transition races).
|
||||
|
||||
Failure-mode cost: a tx span that should span two repo calls but doesn't is a
|
||||
bug — mitigated by the conformance suite (§5 Phase 2) asserting atomicity and
|
||||
check-then-act behavior per command method.
|
||||
|
||||
---
|
||||
|
||||
## 4. How the components work together (interaction docs)
|
||||
|
||||
**A. Agent `run` tool (the core OODA act path).**
|
||||
nomos → MCP client → `mcpserver` run tool → parses args, maps to
|
||||
`ExecutionService.Submit(ctx, target, command, purpose, declaredRisk)`. Submit
|
||||
loads the entity (EntityRepository), resolves the SSH target
|
||||
(TargetResolver), calls `PolicyService.Classify` (policy rules + DB-backed
|
||||
signal context; classifier can only lower autonomy). Read-only /
|
||||
reversible-low → executes now via `CommandExecutor`, streaming output through
|
||||
`ExecutionRepository.AppendLog` and `EventPublisher` (SPA live output);
|
||||
records audit + event; returns exit code. Config-mutation / destructive →
|
||||
persists execution as queued + creates approval, publishes event; returns
|
||||
"awaiting approval". Later the operator clicks Approve in the SPA → REST →
|
||||
`ApprovalService.Decide` → marks approved → `execworker` poller claims the
|
||||
execution (advisory lock) → `ExecutionService.Dispatch` → SSH → results +
|
||||
audit. Both entry paths converge on Submit/Dispatch — one policy, one audit
|
||||
trail.
|
||||
|
||||
```mermaid
|
||||
sequenceDiagram
|
||||
autonumber
|
||||
participant N as nomos (MCP client)
|
||||
participant M as mcpserver run tool
|
||||
participant ES as ExecutionService
|
||||
participant PS as PolicyService
|
||||
participant CE as CommandExecutor (ssh)
|
||||
participant ER as ExecutionRepository
|
||||
participant AP as ApprovalService
|
||||
participant OP as Operator (SPA)
|
||||
participant EW as execworker
|
||||
|
||||
N->>M: run(target, command, purpose, declaredRisk)
|
||||
M->>ES: Submit(target, command, purpose, risk)
|
||||
ES->>PS: Classify(command, declaredRisk)
|
||||
alt read-only / reversible-low
|
||||
ES->>CE: execute now
|
||||
CE-->>ES: exit code + output (streamed)
|
||||
ES->>ER: AppendLog + status + audit
|
||||
ES-->>M: result
|
||||
M-->>N: exit code + output
|
||||
else config-mutation / destructive
|
||||
ES->>ER: create execution (queued)
|
||||
ES->>AP: create approval + event
|
||||
ES-->>M: awaiting approval
|
||||
M-->>N: queued, needs approval
|
||||
OP->>AP: Decide(approval_id, approved)
|
||||
AP->>ER: mark execution approved
|
||||
EW->>ER: Claim next queued (advisory lock)
|
||||
EW->>ES: Dispatch(execution)
|
||||
ES->>CE: execute
|
||||
CE-->>ES: exit code + output
|
||||
ES->>ER: status + logs + audit
|
||||
end
|
||||
```
|
||||
|
||||
**B. Scheduler observe pass.**
|
||||
Timer adapter ticks → `ObservationService.RunPass` → CheckRepository
|
||||
.ListEnabled → group by entity → TargetResolver per check → dispatch to the
|
||||
Checker adapter selected by check kind (each probe is its own adapter; adding
|
||||
a check kind = new adapter + seed row, no core change) → aggregate per-entity
|
||||
health (worst-of + maintenance windows) → SignalRepository.Upsert
|
||||
(open/resolve transitions with signal triggers) → MetricsRepository
|
||||
.InsertSamples → EventPublisher. The SPA receives health changes over SSE.
|
||||
The scheduler package shrinks to: ticker, advisory lock (memory: startup
|
||||
`pg_advisory_lock(0x01c05e6)` on held connection), and pass-loop error
|
||||
handling.
|
||||
|
||||
```mermaid
|
||||
sequenceDiagram
|
||||
autonumber
|
||||
participant T as scheduler (timer adapter)
|
||||
participant OS as ObservationService
|
||||
participant CR as CheckRepository
|
||||
participant TR as TargetResolver
|
||||
participant CK as Checker (probes/*)
|
||||
participant SR as SignalRepository
|
||||
participant MR as MetricsRepository
|
||||
participant EP as EventPublisher
|
||||
participant SPA as web SPA (SSE)
|
||||
|
||||
T->>OS: RunPass
|
||||
OS->>CR: ListEnabled
|
||||
CR-->>OS: enabled check defs
|
||||
loop per check
|
||||
OS->>TR: ResolveForCheck(check)
|
||||
TR-->>OS: SSH target + wrapper
|
||||
OS->>CK: Check(def, target) — adapter chosen by kind
|
||||
CK-->>OS: result (value, state, msg)
|
||||
end
|
||||
OS->>OS: aggregate health (worst-of + maintenance windows)
|
||||
OS->>SR: Upsert signals (open/resolve + triggers)
|
||||
OS->>MR: InsertSamples
|
||||
OS->>EP: Publish health events
|
||||
EP-->>SPA: SSE fan-out
|
||||
```
|
||||
|
||||
**C. Operator creates an entity via the SPA.**
|
||||
REST adapter validates the request shape → `EntityService.Create` →
|
||||
OntologyStore validates type + required attributes → EntityRepository insert →
|
||||
check derivation (moved checkdefaults logic, pure over the TypeTree) →
|
||||
CheckRepository ensure defs → AuditRepository + EventPublisher → response
|
||||
mapped by the REST presenter. The MCP `create_entity` tool calls the same
|
||||
service with its own presenter — one validation path.
|
||||
|
||||
**D. Agent upserts knowledge.**
|
||||
MCP knowledge tool → `KnowledgeService.Upsert` → KnowledgeRepository (row +
|
||||
revision), links `about` edges via RelationshipRepository, dedupes via
|
||||
trigram check → audit + event. REST knowledge endpoints share the service.
|
||||
|
||||
**E. Seed and export (bootstrap/DR).**
|
||||
`oikos seed` CLI adapter → `SeedService.Ingest` reads YAML → repositories
|
||||
upsert; `oikos export` → `SeedService.Export` walks repositories → regenerates
|
||||
`seeds/*.yaml`. The `db` package keeps only connection, migrations, sqlcgen.
|
||||
|
||||
**F. Nomos chat turn (external agent).**
|
||||
nomos is outside the hexagon; its internals use local ports, not `core/ports`.
|
||||
HTTP `/query` or chat bridge → nomos session logic → loads session
|
||||
(SessionStore, local port → pgx) → builds context → `LLMClient` (local port →
|
||||
openai-go) streams → tool calls loop through `HomelabClient` (local port →
|
||||
MCP client → oikos MCP endpoint, flow A) → assistant output persisted. Turn
|
||||
gating, retry caps, message queue are nomos-internal logic; transports and
|
||||
models sit behind nomos's own ports. nomos touches oikos state only via the
|
||||
MCP/REST surface.
|
||||
|
||||
---
|
||||
|
||||
## 5. Phased roadmap
|
||||
|
||||
Each phase ships green: `make lint test generate-check`, `make test-db` where
|
||||
repos change, manual smoke on the dev compose profile. Version bump per repo
|
||||
rules on each merged phase (patch per phase; minor at Phase 9 completion).
|
||||
Deploy from the primary checkout only, tree clean (deploy builds the working
|
||||
tree).
|
||||
|
||||
**Abort criteria (every phase):** if handler contract tests fail without a
|
||||
wire-format explanation, `make test-db` shows changed transaction semantics
|
||||
(atomicity, check-then-act, double-approve protection), or the dev-profile
|
||||
smoke shows behavior drift — revert the phase merge. Do not patch forward
|
||||
through a broken invariant.
|
||||
|
||||
**Phase 0 — ADR + scaffold + dependency rule**
|
||||
1. Write `docs/adr/0016-hexagonal-ports-adapters.md` (context, decision,
|
||||
consequences; references this plan).
|
||||
2. Create `internal/core/{domain,ports,app}` and `internal/adapters/` trees.
|
||||
3. Move `internal/domain` → `internal/core/domain` (mechanical import rewrite).
|
||||
4. Add `depguard` to `.golangci.yml` covering all three §3.1 rules:
|
||||
`internal/core/**` may not import `internal/adapters/**`, `cmd/**`,
|
||||
`internal/nomos`, openai-go, or MCP-client packages. (Rule 3 —
|
||||
`internal/nomos` import bans — activates in Phase 8 when the package
|
||||
exists.)
|
||||
5. Update CONTRIBUTING layout section.
|
||||
|
||||
**Phase 1 — extract the client (web SPA + desktop) into a new repo**
|
||||
|
||||
Grounding: `web/` is a self-contained npm package (`oikos-web`, hand-written
|
||||
API client, vite dev-proxy to :8090/:8092). The build-coupled neighbors move
|
||||
with it: `compose/web/` (Dockerfile + Caddyfile → `oikos-web` image), and
|
||||
`cmd/desktop/` (Wails wrapper — `make desktop` copies `web/dist` into the
|
||||
binary and its auto-update reads `dtoro/oikos` releases,
|
||||
`cmd/desktop/main.go:41`).
|
||||
|
||||
Decisions (settled in review): full UI delivery stack moves; new repo gets
|
||||
its own webhook-triggered deploy pipeline and its own compose project on the
|
||||
mac-mini; this runs before the hexagon phases so the backend refactor and its
|
||||
Phase 9 doc rewrite land once on a backend-only tree.
|
||||
|
||||
1. Create `git.hubris.network/dtoro/oikos-web` (matches the npm package
|
||||
name). Fresh git history; the oikos repo retains the old history. Copy:
|
||||
`web/`, `cmd/desktop/` (as `desktop/`), `compose/web/` (Dockerfile +
|
||||
Caddyfile, adjusted build context), and the `ui` / `desktop` /
|
||||
`desktop-package` / `install` Makefile targets.
|
||||
2. New repo gets its own `VERSION` file with the same bump-on-main rule. The
|
||||
web Dockerfile's `COPY VERSION ./` and vite's VERSION read now resolve
|
||||
inside the new repo: the SPA sidebar shows the UI repo's version; the
|
||||
backend version remains available via the API/MCP ping.
|
||||
3. New repo CI mirrors the current `.gitea` `web` job (npm lint / typecheck
|
||||
/ test / build) plus a desktop build job.
|
||||
4. New repo deploy: Gitea webhook → deploy script mirroring `deploy.sh`
|
||||
essentials (CI-green gate, version-tagged `oikos-web:v$VERSION`, prune to
|
||||
3 newest tags, builds the working tree — same constraint as oikos deploys).
|
||||
Own minimal `docker-compose.yml` publishing `8091:80` with the same
|
||||
mem/cpu limits and restart policy. Second webhook receiver + launchd unit
|
||||
on the mac-mini mirroring `cmd/webhook`, or one more route on the existing
|
||||
receiver — implementer's choice.
|
||||
5. Cutover on mac-mini, in order: stop and remove the old `web` service from
|
||||
the oikos stack (frees host port 8091) → bring up the new compose project
|
||||
→ verify the outer Caddy (LXC 121) still serves `oikos.hubris.network`:
|
||||
SPA fallback, Authentik flow, `/api`+`/mcp`+`/agent` split. No Caddy
|
||||
changes expected — routing targets the published port, not a Docker
|
||||
network.
|
||||
6. Strip from oikos: `web/`, `cmd/desktop/`, `compose/web/`, the
|
||||
ui/desktop/deploy-ui Makefile targets, the CI `web` job, the `web`
|
||||
service in `docker-compose.yml`, and `oikos-web` from `deploy.sh`'s build
|
||||
and prune lists. Update README / CONTRIBUTING / AGENTS.md layout sections
|
||||
to point at the new repo. Historical `plans/` and `docs/adr/` references
|
||||
stay as-is (append-only convention).
|
||||
7. Desktop auto-update: `updateURL` in the desktop main changes to
|
||||
`dtoro/oikos-web` releases. Tag the first new-repo release ≥ the last
|
||||
oikos desktop version so the updater sees an upgrade. Existing installed
|
||||
desktop builds keep checking the old repo and will stop finding updates —
|
||||
one manual reinstall for the single operator; note it in the desktop
|
||||
release notes.
|
||||
8. Rollback: both sides keep versioned images. If the new pipeline fails,
|
||||
stop the new project, and a pre-split oikos checkout can re-up its `web`
|
||||
service and reclaim 8091.
|
||||
|
||||
Phase 1 acceptance: SPA served end-to-end from the new pipeline with auth and
|
||||
SSE intact; desktop app builds and updates from the new repo; `oikos` CI
|
||||
green with no web job; an UI-only commit deploys without touching the
|
||||
backend stack; `oikos-web` images absent from the oikos deploy prune list.
|
||||
|
||||
**Phase 2 — ports package + conformance wrappers**
|
||||
1. Define driven-port interfaces in `internal/core/ports` (§3.3) against
|
||||
`core/domain` types only.
|
||||
2. Move `secrets.Backend` interface → `ports.Secrets`; adapters keep impls;
|
||||
delete mcp's local `secretBackend`.
|
||||
3. Postgres adapter: repository structs wrapping existing pool + sqlcgen
|
||||
(move `internal/db` → `adapters/postgres`; queries dir moves with it,
|
||||
sqlc.yaml path updated, `make generate` verified).
|
||||
4. ssh adapter wraps actuator functions behind `CommandExecutor`; remote
|
||||
adapter implements `TargetResolver` on `EntityRepository`.
|
||||
5. Add `internal/core/ports/ports_test` fakes (in-memory repos, recording
|
||||
executor, fake checker, spy publisher) for service tests.
|
||||
|
||||
**Phase 3 — first vertical slice: entities + graph + composition root**
|
||||
1. Implement `EntityService`, `GraphService`, `MonitoringService` (absorb
|
||||
checkdefaults into core).
|
||||
2. Rewire `httpapi` entities/relationships/ontology/graph/checks handlers and
|
||||
`mcpserver` entity/graph tools to the services; delete their inline SQL.
|
||||
3. `cmd/oikos/main.go` becomes the composition root (§3.5); main owns the chi
|
||||
router and mounts REST + `/mcp`.
|
||||
4. Port `mutations_test.go` / `api_test.go` entity cases to service-level
|
||||
tests with fakes; keep handler contract tests.
|
||||
|
||||
**Phase 4 — governance + execution slice (highest value)**
|
||||
1. Implement `PolicyService`, `ExecutionService`, `ApprovalService`,
|
||||
`AuditService`, `SecretsService`.
|
||||
2. Dismantle `classifyAndGate`: tool handler → arg mapping → Submit; policy
|
||||
rules → PolicyService; approval creation → ApprovalService; SSH dispatch →
|
||||
ExecutionService.Dispatch via CommandExecutor.
|
||||
3. `execworker` becomes a poller adapter calling ExecutionService; `execlog`
|
||||
folds into the execution-log repository + EventPublisher adapter
|
||||
(identical throttling/SSE behavior).
|
||||
4. Rewire `httpapi` executions/approvals/classifications/risk-classes/
|
||||
autonomy/audit/events/activity handlers and mcp ops tools.
|
||||
5. Preserve `idempotency_keys` semantics across the two converged paths.
|
||||
|
||||
**Phase 5 — observation slice**
|
||||
1. Split `scheduler.go`: `ObservationService` + `SignalService` in core; one
|
||||
probe adapter per check kind under `adapters/probes/`; `Checker` registry
|
||||
keyed by check kind. The probe concurrency cap moves with it — `RunPass`
|
||||
keeps the bounded worker pool of 10 (`scheduler.go:133`).
|
||||
2. `MetricsRepository` + health aggregation move behind ports; staleness
|
||||
sweep in service; timer + advisory lock stay in the scheduler adapter.
|
||||
3. Rewire `httpapi` signals/fleet-health/dashboard handlers.
|
||||
|
||||
**Phase 6 — knowledge + learning slice**
|
||||
1. `KnowledgeService` (search, upsert, revisions, tags, merge, drift) shared
|
||||
by REST + MCP knowledge tools.
|
||||
2. `LearningService` with Pattern/Feedback repositories; keep ≥80% coverage
|
||||
gate on the moved logic.
|
||||
|
||||
**Phase 7 — seeds, provisioning, inversions**
|
||||
1. `SeedService` absorbs `db/seed.go` + `db/export.go`; `db` package reduces
|
||||
to connection/migrations/sqlcgen inside the postgres adapter — the
|
||||
`db → checkdefaults` edge is gone.
|
||||
2. `ProvisioningService` + `Provisioner` port for pct/qm flows.
|
||||
3. CLI subcommands become adapters over SeedService/SecretsService.
|
||||
|
||||
**Phase 8 — nomos internal cleanup (no hexagon) + leaves**
|
||||
1. Extract `cmd/nomos` logic into `internal/nomos` plain packages: `session`,
|
||||
`turngate`, `retrycap`, `messagequeue`, `assent` (keep existing unit tests
|
||||
moving with them).
|
||||
2. Define nomos-local ports in `internal/nomos`: `LLMClient`, `HomelabClient`
|
||||
(MCP client), `SessionStore`. `cmd/nomos/main.go` becomes the composition
|
||||
root wiring openai-go, the MCP client, and the pgx store into those ports.
|
||||
3. depguard rules 2–3 from §3.1 go live: `internal/core` bans
|
||||
`internal/nomos`, `openai-go`, MCP-client packages; `internal/nomos` bans
|
||||
`internal/core` and `internal/adapters`.
|
||||
4. Document `webhook` and `desktop` as leaf utilities in ADR-0016 (no
|
||||
restructuring).
|
||||
|
||||
**Phase 9 — cleanup + docs + gates**
|
||||
1. Delete dead code paths and the old package locations; full depguard audit
|
||||
(zero core→adapter imports).
|
||||
2. Coverage gates: `ExecutionService` and `PolicyService` ≥ 90% each (the
|
||||
safety-critical pair), plus a gating-matrix test — risk class × autonomy
|
||||
mode × declared risk → outcome (auto-run / queue / deny) asserted as a
|
||||
table, since line coverage alone cannot prove the classifier. Keep
|
||||
existing gates (policy + learning ≥ 80%, others ≥ 60%).
|
||||
3. Update README, CONTRIBUTING, AGENTS.md layout sections; bump minor
|
||||
version; deploy.
|
||||
|
||||
---
|
||||
|
||||
## 6. Risks and mitigations
|
||||
|
||||
| Risk | Mitigation |
|
||||
|---|---|
|
||||
| Behavior drift while extracting services | No wire-format changes; existing handler tests (`api_test.go`, `phase3_test.go`, `mutations_test.go`, `pct_create_test.go`) keep running against adapters; new service tests with fakes cover logic before rewiring |
|
||||
| Import churn breaks CI for days | One phase per merge, mechanical moves, `make generate-check` after sqlc path moves |
|
||||
| Dual execution paths (immediate vs queued) diverge | Both converge on ExecutionService.Submit/Dispatch in Phase 4; idempotency_keys behavior asserted by test |
|
||||
| SSE/execution-log streaming regressions | EventPublisher adapter keeps broker + throttling behavior; `sse_test.go` unchanged |
|
||||
| depguard false positives during migration | Rule tightened per phase (initially warn-only on already-moved packages, deny at Phase 9) |
|
||||
| Coverage gates dip mid-refactor | Adjust per-phase in CI config, restore at Phase 9 |
|
||||
| UI split breaks serving or auth | Port-8091 cutover is ordered (old service down before new up); outer Caddy untouched — it targets the published port; versioned-image rollback on both sides re-ups the old web service |
|
||||
|
||||
## 7. Validation
|
||||
|
||||
- Per phase: `make lint test generate-check`; `make test-db` for repository
|
||||
changes; `docker compose --profile dev up` + smoke: entity CRUD via REST and
|
||||
MCP, one observe pass, one gated execution end-to-end.
|
||||
- Phase 1 acceptance: SPA served from the new repo's pipeline at
|
||||
`oikos.hubris.network` with Authentik flow and SSE intact; desktop app
|
||||
builds from the new repo; oikos CI green without the web job.
|
||||
- Phase 4 acceptance: MCP `run` read-only executes, config_mutation queues an
|
||||
approval, approval via REST resumes execution — all observable in SPA.
|
||||
- Phase 9 acceptance: `rg "internal/adapters" internal/core` returns nothing;
|
||||
ADR + docs updated; deployed via `deploy.sh` from clean tree.
|
||||
|
||||
## 8. Out of scope / open decisions
|
||||
|
||||
- No DB schema changes, no API/MCP contract changes, no SPA behavior changes,
|
||||
no binary-merging (nomos stays a separate deployable).
|
||||
- Decided (review): UI extraction = Phase 1 — `web/`, `cmd/desktop/`,
|
||||
`compose/web/` move to `dtoro/oikos-web` with their own pipeline, compose
|
||||
project, and VERSION.
|
||||
- Presenter strategy: REST and MCP keep separate thin presenters (recommended,
|
||||
default) rather than shared DTOs.
|
||||
- Decided: one hexagon. nomos is an external agent client with a Phase 8
|
||||
internal cleanup (local ports, no `core/adapters` tree). Revisit only if
|
||||
nomos grows a second consumer of its session logic.
|
||||
196
plans/done/2026-08-15-session-audit-arr-improvements.md
Normal file
196
plans/done/2026-08-15-session-audit-arr-improvements.md
Normal file
@@ -0,0 +1,196 @@
|
||||
# Oikos Improvement Plan — Session Audit 2026-08-15
|
||||
|
||||
**Reviewed 2026-08-16 — Status: Won't do** — superseded by hexagonal refactor; re-evaluation needed.
|
||||
|
||||
## Session context
|
||||
|
||||
A 4-hour session to fix the arr-media automation pipeline on the `arriman` LXC.
|
||||
The agent:
|
||||
|
||||
- Used the **terminal tool** (raw SSH) exclusively — no `run` calls
|
||||
- Used **Infisical** via `set_secret` (worked well)
|
||||
- Did **not** consult the knowledge graph before touching entities
|
||||
- Did **not** register ephemeral entities (nzbhydra2) in the graph
|
||||
- Did **not** log mutations (Caddy changes, docker-compose edits, DNS records)
|
||||
- Did **not** write knowledge entries for discoveries made
|
||||
|
||||
## Root cause analysis
|
||||
|
||||
The agent's default behavior is to use whatever tool provides the fastest path.
|
||||
Raw SSH (`terminal`) is faster than `mcp__oikos__run` because:
|
||||
|
||||
1. **`run` gates behind approval** — even read-only commands on LXCs touching `/etc/` or `/opt/` escalate from `read_only` to `config_mutation` (transport-aware escalation, `server.go:770`). The agent didn't know this and assumed `run` would be slower/blocked.
|
||||
|
||||
2. **`run` requires a plan-first gate** — every `run` in a Nomos session checks for a plan before executing (`server.go:795`). The agent had no plan registered.
|
||||
|
||||
3. **No docker-exec native support** — common *arr operations (`docker exec <container> curl ...`) don't map cleanly to Oikos targets. The agent would have needed to do `run(target="lxc:arriman", command="docker exec prowlarr curl ...")`, which is verbose and classifies as config_mutation (touching container internals).
|
||||
|
||||
4. **No "batch run" or pre-authorization** — the agent made ~200+ SSH calls. With `run` each one would individually classify, deduplicate, create execution rows, and potentially queue for approval. Raw SSH just ran.
|
||||
|
||||
5. **Graph drift is invisible at session end** — there's no automatic `audit_knowledge_graph` trigger or summary that says "you changed X, Y, Z — update the graph."
|
||||
|
||||
---
|
||||
|
||||
## Improvement plan
|
||||
|
||||
### P0: Docker-exec execution primitive
|
||||
|
||||
**Problem:** The most common operation in *arr management is `docker exec <name> <cmd>`. Currently requires `run(target="lxc:arriman", command="docker exec prowlarr curl ...")`, which is verbose and classifies as config_mutation.
|
||||
|
||||
**Proposed:** Add a `docker_exec` tool:
|
||||
|
||||
```
|
||||
mcp__oikos__docker_exec(lxc_slug, container, command, risk)
|
||||
```
|
||||
|
||||
- `lxc_slug`: the LXC entity (e.g. `lxc:arriman`)
|
||||
- `container`: the Docker container name (e.g. `prowlarr`, `sonarr`)
|
||||
- `command`: what to run inside the container
|
||||
- `risk`: self-declared risk (default read_only for curl/status)
|
||||
|
||||
Implementation:
|
||||
- Resolves the LXC slug to SSH host/user via existing `resolveHost`
|
||||
- Wraps the command as: `docker exec <container> sh -c '<escaped_command>'`
|
||||
- Classifies with the same `policy.ClassifyCommand` but with a **lower default** (read_only for curl queries, `docker exec <name> cat/config/status`)
|
||||
- Records a `docker_exec` execution type so the audit trail distinguishes from raw `run`
|
||||
|
||||
**Files to change:**
|
||||
- `internal/mcp/ops_tools.go` — add `dockerexec_tool` registration
|
||||
- `internal/policy/classify.go` — add docker-exec-specific command patterns
|
||||
|
||||
**Estimate:** 1 day
|
||||
|
||||
---
|
||||
|
||||
### P1: Session-scoped trust (assent window expansion)
|
||||
|
||||
**Problem:** Agents make 200+ SSH calls in a session. The approval model gates each one individually, forcing agents to bypass Oikos.
|
||||
|
||||
**Proposed:** Expand the existing "assent window" pattern (`server.go:839`) from plan-level to session-level:
|
||||
|
||||
1. Agent calls `propose_plan` (already exists) — operator approves
|
||||
2. All subsequent `run` calls within the same session auto-approve for the scope declared in the plan
|
||||
3. Commands outside the plan's scope still require new approval
|
||||
|
||||
**Implementation:**
|
||||
- Track the plan's declared scope (entity slugs + risk classes) in `nomos_plan_executions`
|
||||
- `classifyAndGate` checks: is `sessionID` in an approved plan whose scope covers this target + risk class?
|
||||
- If yes → auto-execute, record audit entry referencing the plan approval
|
||||
|
||||
**Estimate:** 2 days
|
||||
|
||||
---
|
||||
|
||||
### P2: Post-session drift summary
|
||||
|
||||
**Problem:** After 200+ tool calls, the knowledge graph is stale. There's no signal that the agent should update it.
|
||||
|
||||
**Proposed:** A new MCP tool `upsert_session_summary`:
|
||||
|
||||
```
|
||||
mcp__oikos__upsert_session_summary(session_id, summary, entities_touched[], mutations[], discoveries[])
|
||||
```
|
||||
|
||||
Called at session end by the agent. Auto-creates:
|
||||
- Knowledge entries for each discovery
|
||||
- Entity attribute updates for changed facts
|
||||
- Relationship records for new edges discovered
|
||||
- Audit trail entries linking everything to the session
|
||||
|
||||
**Implementation:**
|
||||
- Wraps existing `upsert_knowledge`, `update_entity_attributes`, `create_relationship`
|
||||
- Batch upsert in a single transaction
|
||||
- Returns a report: "Created 3 knowledge entries, updated 5 entity attributes, added 2 relationships"
|
||||
|
||||
**Estimate:** 1.5 days
|
||||
|
||||
---
|
||||
|
||||
### P3: Docker-container entity type + drift detection
|
||||
|
||||
**Problem:** NZBHydra2 was deployed, used, and removed — the graph never knew it existed. Docker containers on the arriman LXC (prowlarr, sonarr, radarr, lidarr, sabnzbd, etc.) have no entity representation.
|
||||
|
||||
**Proposed:** Add a `container` entity type to the ontology:
|
||||
|
||||
```yaml
|
||||
- name: container
|
||||
label: "Docker Container"
|
||||
parent: service
|
||||
attributes_schema:
|
||||
image: string
|
||||
port: integer
|
||||
network_mode: string
|
||||
```
|
||||
|
||||
And extend `discover_infra_drift` to detect Docker-container-to-entity drift:
|
||||
|
||||
1. Query `docker ps --format '{{.ID}} {{.Names}} {{.Image}}'` on LXCs with role `docker-host`
|
||||
2. Compare against `container:*` entities in the DB
|
||||
3. Report: containers with no entity (ghost) and entities with no running container (missing)
|
||||
|
||||
**Files to change:**
|
||||
- `seeds/ontology.yaml` — add `container` type
|
||||
- `internal/mcp/discover.go` — add docker drift detection
|
||||
- `internal/mcp/entity_tools.go` — auto-derive container checks from `monitoring: ["http", "docker"]`
|
||||
|
||||
**Estimate:** 2 days
|
||||
|
||||
---
|
||||
|
||||
### P4: Knowledge-graph-first agent instruction
|
||||
|
||||
**Problem:** The session never consulted Oikos for entity topology. The agent's SOUL.md (HERMES.md) doesn't instruct it to do so.
|
||||
|
||||
**Proposed:** Add a mandatory step to the homelab agent persona (`/opt/homelab-context/HERMES.md`):
|
||||
|
||||
```
|
||||
## Before any terminal/SSH command
|
||||
|
||||
1. Check Oikos knowledge graph: `list_entities(type="lxc")` + `get_entity(slug="...")`
|
||||
2. If the entity exists → use `run` not raw SSH
|
||||
3. If it doesn't exist → create it with `create_entity`
|
||||
4. After changes → update attributes with `update_entity_attributes`
|
||||
```
|
||||
|
||||
**Estimate:** 0.5 days (documentation-only)
|
||||
|
||||
---
|
||||
|
||||
### P5: DNS management tools
|
||||
|
||||
**Problem:** Session had to write custom Python to interact with Technitium API (token redaction, shell escaping). This is common enough to warrant native tools.
|
||||
|
||||
**Proposed:** Add DNS management tools:
|
||||
|
||||
```
|
||||
mcp__oikos__add_dns_record(zone, domain, type, value, ttl)
|
||||
mcp__oikos__delete_dns_record(zone, domain, type, value)
|
||||
mcp__oikos__list_dns_records(zone, filter)
|
||||
```
|
||||
|
||||
Implementation wraps the Technitium HTTP API behind the MCP layer, handling token authentication automatically via Infisical secrets.
|
||||
|
||||
**Estimate:** 1 day
|
||||
|
||||
---
|
||||
|
||||
## Priority matrix
|
||||
|
||||
| # | Item | Effort | Impact | Risk |
|
||||
|---|---|---|---|---|
|
||||
| P0 | Docker-exec tool | 1d | High — single biggest friction point | Low |
|
||||
| P1 | Session-scoped trust | 2d | High — removes approval barrier | Medium (auth model change) |
|
||||
| P2 | Post-session drift summary | 1.5d | Medium — closes the feedback loop | Low |
|
||||
| P3 | Container entity + drift | 2d | Medium — fills a blind spot | Low |
|
||||
| P4 | Agent instruction update | 0.5d | High — behavioral change | None |
|
||||
| P5 | DNS management tools | 1d | Low — but saves time every session | Low |
|
||||
|
||||
## Recommended execution order
|
||||
|
||||
1. **P0 + P4** (1.5 days) — Remove the biggest friction point + update instructions
|
||||
2. **P1** (2 days) — Make Oikos the preferred execution pathway
|
||||
3. **P2** (1.5 days) — Ensure knowledge doesn't leak out of sessions
|
||||
4. **P3** (2 days) — Fill the Docker blind spot
|
||||
5. **P5** (1 day) — DNS nicety
|
||||
|
||||
Total: ~8 days of work for a complete session-to-graph feedback loop.
|
||||
Reference in New Issue
Block a user